Code Review As A Gateway

We’ve talked a lot about what to look for in a Code Review, but we haven’t covered the process at all. There are many different reasons to perform code reviews in your team or organisation, and the process you choose should be driven by what you’re trying to achieve with the reviews.

The first example we’re going to explore is Code Review as a Gateway. In this process, a developer picks up or is assigned a task, implements the code to the best of his/her ability, and when s/he’s “finished”, submits it for review. The reviewer checks the code and, if everything is OK, the code is merged into the main development branch.

This was a fairly typical approach in organisations with a formal software development process – each stage had a gateway which needed one or more people to sign off to say the work had been done correctly. In these days of more collaborative, agile approaches, however, this is less common, particularly if the team is colocated. But it is a similar process to that of GitHub pull requests, so that’s the example we’re going to explore.

The process

In this example, we’re working on an open source project hosted on GitHub. I have decided to pick up one of the issues listed. Generally it’s a good idea to mention on the mailing list, or discuss with the project owner that you want to work on this, and maybe have a conversation on the approaches you want to take. The project owner may be able to point you to some example code that fits the pattern they want, and/or point out some gotchas to be aware of.

I’ll go away and implement the fix or feature. If I have doubts I’ll probably chat to the owner either on the ticket or via chat/email, but eventually I’ll submit a pull request with what I think is the complete working code.

GitHub pull request

In Upsource, pull requests are automatically converted into Code Reviews:

New Code Review from Pull Request

At this point, the project owner reviews the whole thing with one question in mind – is it good enough to merge? Usually there will be some comments, questions or feedback from the reviewer. Like GitHub, Upsource lets you leave these comments either as a review level comment.

Review Level Comment

Or at the code level

Code Level Comment

Comments in Upsource are automatically synched with GitHub and vice versa, so reviewers are free to comment in either tool.

Comments are synced with GitHub

As the code author, it’s my job to look at the comments and either implement suggestions, defend my decisions, or ask questions. In Upsource, comment labels can help clarify whether this is something that really needs to be addressed before the code is merged, or if it’s simply a question to be answered or a comment to note.

Labels for comments

If there are comments that need to be addressed by updating the code, then I will make the changes and then add them to this code review.

Make code changes

It’s possible that we then begin a cycle of suggesting improvements or additions and receiving updated code. In theory, on each iteration there are fewer comments, and fewer/smaller changes that need to be made. Finally, the reviewer should be happy enough with the code to merge it, closing the pull request.

Merge

You can even merge from Upsource

Merge via Upsource

Discussion

Because this is a gateway type of process, often the reviewer is more senior, or at least in a position to have final veto over the code. Therefore, during a code review of this type, the author is often defending their design and implementation, and the reviewer is trying to think of reasons why this should not be merged into the main code base.

This can lead to a certain level of antagonism: as the code author, you’ve already invested time writing the code, you’ve thought it through to the best of your ability, and you have a solution that you think works; as a reviewer, it seems that your goal is to think of everything that could be wrong with the code before it is merged in, so you can prevent bad/incorrect/suboptimal code sneaking into your code base.

Many of us have been through these sorts of reviews, and I suspect it’s a reason people react negatively to the idea of code reviews – it feels like a combative experience where no one wins.

This type of review process can work well, however, when the goal of the reviewer is not to prevent bad code from going into the code base, but to add functionality to the project/application, i.e. rewarding reviewers on how quickly they merge in the code. Of course, the danger here is that reviewers will not look at the code properly and simply say “yes”. But given how much can be automated (compilation, tests, inspections, static analysis, performance, security etc), in this kind of review the emphasis could be on making sure the right checks have been automated for this code (e.g. ensuring unit / functional / performance / security tests exist) and ensuring that those checks are testing the right things.

What gateway reviews are really horrible for is checking the design of the code. Really it’s too late at that stage – either the poor author has to throw everything away and start again (is there anything more demoralising as a developer?) or the code needs to be merged in even with its non-standard, non-optimal design. If design and architecture matter to your team and your application, these things need to be discussed before starting the implementation. A gateway code review then simply checks the code follows the ideas discussed, and if it doesn’t, the reviewer assumes the code author had a good reason for this, and perhaps starts a discussion about why the original design expectations didn’t match what was actually implemented.

Let’s distill some of this discussion into the pros and cons of this approach to code reviews.

Advantages

  • Someone on the team is ensuring that code that’s merged into the main code base meets the required levels of quality (however that is measured).
  • If the process is geared towards accepting, rather than rejecting, code1, these types of reviews can be relatively quick and straightforward.
  • If the process is geared towards accepting code, and either expectations around code style/approaches are very clear or a certain amount of inconsistency within the code base can be tolerated, this process can encourage contributions from a wider community of developers (e.g. open source projects on GitHub)

Disadvantages

  • The major disadvantage is that the work has already been done. So either the reviewer needs to recognise that the work has been implemented and therefore focus their feedback on only preventing major problems from creeping in, or the code author needs to accept that re-work from a code review could be extensive.
  • If the code review is focussed on getting the code absolutely correct2, each iteration of comments/code changes can be fairly long, and there can be quite a lot of these iterations.
  • Handing code over to be reviewed blocks progress, so developers following this kind of process often have more than one fix/feature in progress, sometimes several. A lot of work-in-progress and a lot of context switching can be detrimental to a team’s productivity.
  • This is not the best way to up-skill developers. Suggestions given once the code is written can be seen as “you did this wrong” instead of “there’s a nice way of doing this thing”, and developers are not as receptive to these suggestions as those received while solution is being created.

Anti-patterns

When gateway code reviews are focussed on getting the code absolutely correct2, these long and plentiful cycles of review/change/re-review/more-changes take longer than the initial code. That makes authors and reviewers dread the death-march of the code review – authors leave it longer before review, meaning much more code goes into a review, and reviewers wait longer before opening and commenting on reviews, both of which lead to even longer reviews. Without clear guidelines on what a reviewer is supposed to be looking for and what is (and is not) a showstopper, even the reviewer can be unable to bring this cycle to a satisfactory end.

To overcome these problems:

  • Code reviews should contain a small set of changes that’s easy to review.
  • Reviewers should have a clear set of guidelines on what they should be looking for.
  • The goal should be to accept the code unless a showstopper is identified.
  • Code can be annotated by the author in advance in the code review tool to explain approaches or justify design decisions up front to preempt questions.

Also remember to praise good solutions and nice code, as well as drawing attention to potential problems. This can alleviate the feeling that code reviews are all about criticism.

Suitable for

  • Teams with high levels of trust with a shared vision of the code – it’s much easier to accept the code of developers you trust.
  • Code bases that don’t need strict control over the design and can tolerate multiple different programming approaches – it’s too late to discuss design or architecture in a gateway code review.
  • Teams with alternative channels to discuss design and architecture, e.g. via mail, chat, issue tracker, or even in person, and a process that either prevents implementation before a design is agreed, or actively allows collaboration and feedback during implementation.
  • Projects where waste can be tolerated – if it’s OK to throw away contributions. This way an initial “reject” won’t lead to lots of explanations and re-work, but possibly to an alternative contribution from a different developer.

 

1 Note: a good way to achieve this is to make sure everyone is clear and consistent with what they’re looking for during the review, and what they consider to be a showstopper.

2 For some definition of correct, from the reviewer’s point of view.

Posted in Code review practices, Newsletter | Tagged , | 1 Comment

Upsource 3.5 (Build 3616) Is Available

Hi Everyone!

happyholiday_02

Christmas time is just around the corner, and we have a nice little surprise for you: welcome a fresh Upsource 3.5.3616. This update brings a number of bug fixes, such as star/unstar project issue, missing diff summary in some reviews, etc.

Check the Release Notes for more details.

Download the latest Upsource 3.5.3616 and enjoy all the new goodies it brings today.

P.S. We wish you a merry Christmas and a very happy and productive New Year!

Posted in Newsletter, Release, Update | Tagged , | Leave a comment

Upsource 3.5.3597 bug-fix update is available

Hi everybody!

A new Upsource update is now available with a number of fixed issues including a couple of critical ones. See the complete list of changes here.

fixed-issues-3597

Download Upsource 3.5.3597 build here, and don’t forget to backup your current instance before upgrading.

Yours truly,
The Upsource Team

Posted in Update | Leave a comment

PHP code insight in Upsource 3.5

In the recent release, Upsource 3.5, we have significantly improved PHP code intelligence features announced in the previous release. We’ve added support for composer dependency manager and made it possible to explicitly specify external dependencies. In Upsource 3.5 you can now set the PHP language level, and fully enjoy smart code review features.

In the following video Gary Hockin, a Developer Advocate for PhpStorm at JetBrains, explores Upsource and its PHP code insight. Check it out!

Posted in Feature | 1 Comment

New Upsource 250-user pack

Hi everyone!

We have some important news to share today. We have extended the list of available Upsource licensing options with a new 250-user pack.

Previously, the next available option after 100-user pack was 500-user pack. The gap between these two options, both in terms of the price and the number of users, was too vast. It made it harder for some companies to adopt Upsource even if they felt the tool was right for them.

To address this issue we have decided to introduce a new option – Upsource 250-user pack, that should accommodate for more use cases and give more flexibility to companies of different sizes.

The new 250-user pack is already available for purchase at Upsource’s Buy page, as well as the option to upgrade your current instance to it.

Should you have any questions about the new user pack, pricing or purchasing Upsource, please do contact us at sales@jetbrains.com.

Yours truly,
The Upsource Team

Posted in Announcements | Leave a comment

Upsource 3.5.3550 bug-fix update is available

Hi everybody!

We have an Upsource update for you today. It addresses a number of issues in various areas – see the complete list of changes here.

screen-shot-2016-11-03-at-14-14-39

Download Upsource 3.5.3550 build here, and don’t forget to backup your current instance before upgrading.

Yours truly,
The Upsource Team

Posted in Update | Leave a comment

Distributed Upsource Installation

We are delighted to announce that with the latest release Upsource has reached an entirely new level of scalability. In the last few months, we have revised and reworked Upsource’s architecture. As a result of this work, we are now able to provide a distributed cluster installation that is highly scalable, fault-tolerant and available 24/7.

This new multi-node cluster installation allows you to scale and distribute Upsource services to different nodes and machines providing more control and resources demanded by large projects and thousands of users.

scalable_scheme

The setup procedure is not as simple as with the standard distribution, but we are confident that if you follow our instructions, you will succeed with it. Having said that, if you do need any assistance, or simply have questions, do not hesitate to reach out to our support team at upsource-support@jetbrains.com.

Yours truly,
The Upsource Team

Posted in Feature | Leave a comment

Webinar recording: “What’s New in Upsource 3.5”

The recording of our webinar “What’s New in Upsource 3.5” is now available on JetBrains YouTube channel.

In this webinar Trisha gives an overview of the new features and major improvements introduced in the latest release, Upsource 3.5.

About presenter:

Trisha_Gee
Trisha has developed Java applications for a range of industries, including finance, manufacturing and non-profit, for companies of all sizes. She has expertise in Java high performance systems, is passionate about enabling developer productivity, and dabbles with Open Source development.

Timeline:
0:50 Redesigned home page
2:00 Search box on the home page
2:30 New Search features
3:55 File revision data
5:10 Side by side diff improvements
6:04 User roles in a project
7:20 User profiles
8:58 Smarter notifications
10:00 Annotated changes
11:05 Updated IntelliJ IDEA engine
12:05 Reviewers graph
14:45 Images in comments
16:30 Clone repositories
17:15 PHP support enhancements
17:50 New IDE integration features
25:27 Updated custom workflows
28:37 New webhooks
28:53 Updates to issue tracker integration
29:15 Performance and scalability

P.S.: We have received a few questions about Upsource API and its documentation. You can find the link to the API documentation and usage examples in your Upsource instance, in the bottom-right corner of the web UI.

Posted in Webinars | Leave a comment

Live webinar: What’s New in Upsource 3.5

If you would like to see the new Upsource 3.5 features in action, make sure you don’t miss out live webinar! Join us Thursday, October 27th, 10:00 AM-11:00 AM EDT (16:00 – 17:00 CEST). Things that we’re going to cover include, but are not limited to the following:

  • Redesigned home page
  • Public user profiles
  • Omni-search
  • Revamped custom workflows
  • Smarter notifications
  • Reviewers graph
  • Annotated changes
  • IDE Integration Improvements

upsource_webinar_what_s_new_in_upsource_3_5

Space is limited, please register now.

This webinar is geared towards developers of different proficiency regardless of programming language of choice. During the webinar there will be an opportunity to ask questions. The recording will be available after the webinar.

About presenter:

Trisha_Gee
Trisha has developed Java applications for a range of industries, including finance, manufacturing and non-profit, for companies of all sizes. She has expertise in Java high performance systems, is passionate about enabling developer productivity, and dabbles with Open Source development.

Posted in Webinars | Leave a comment

Upsource 3.5 is released!

Today we have released Upsource 3.5, a major update loaded with new features and various improvements. This release aims to make your overall code review process more productive and enjoyable. Here’s what we’ve included in this build:
up35-homepage

  • Redesigned home page: The new Home page is a lot more flexible and can be equally friendly to those who have a dozen of projects as well as to those who have hundreds of projects. Plus it provides more useful information on the activity in the projects.
  • Brand new user profile pages: Upsource now lets you know more about your teammates, and see what they’re working on, what projects they contribute to, and so on. From your own profile page, you can quickly access your commits, reviews and other information.
  • Omni-search: Upsource’s search is now capable of finding files across projects, projects themselves and even people, and it has become sensitive to the context you’re in. Unfortunately, we also need to warn you that this build has one particular regression – full text search does not work in Upsource 3.5. We apologize for the inconvenience, and we will do our best to release an update with this feature properly working as soon as possible.
  • Revamped custom workflows: Previously introduced Custom workflows have been reworked as well, and allow a lot more flexibility.
  • Reviewers graph: With this new addition to the Analytics section you can now learn more about interactions in your projects and see what code review relationships have been built.
  • IDE integration improvements: As usual IDE integration has been populated with even more features to help you do more without leaving your IDE.
  • Annotated changes: When reviewing a branch with contributions from several people, it helps to have an annotated view of the changes, to see who wrote what.
  • File revision data: Am I looking at the latest revision of a file? Are there modifications that haven’t been merged to master yet? Wonder no more! Now this information is provided for every file at every revision.
  • Updated IntelliJ IDEA engine: This means you get to enjoy the newest IntelliJ IDEA inspections in Upsource as well.
  • Smarter notifications: If a code review completely slipped your mind, Upsource will now remind you to address pending changes.

If you’d like to learn more about the new Upsource 3.5 features, please check out the What’s New page or register to our free webinar “What’s New in Upsource 3.5” with Trisha Gee.
upsource_webinar_what_s_new_in_upsource_3_5

To try Upsource 3.5 download the build and don’t forget to backup your current instance!

Posted in Release | Leave a comment