Upsource Blog

The last three posts on code review workflows show that there’s more than one way to do a code review, and that the approach you take will depend upon what you want to achieve with your reviews and the type of team you’re working with.

In this post, we’ll cover how Upsource aims to make your workflow, whatever it looks like, simpler by automating as much of it as possible. We’ll show the types of things that can be easily automated, when this might be useful, and show how to achieve it.

Automatically Create Reviews

If your workflow is predictable in who reviews code, you don’t need to manually create reviews and add reviewers, you can have Upsource do this for you. In this example, we’ll assume that you have a single individual who reviews all code that is committed.

Steps:

This is achieved using Custom Workflows.

1. Log in as an administrator and go to the administration page by clicking on the settings icon
2. Select the project you want to enable automatic code reviews for, and click its Edit project button.
3. Click on the Custom workflows link on the right of the page.
4. Under the “Create reviews automatically” heading, click the Enable check box, then click the Add trigger button that appears.
5. For our specific example, we’re not going to enter any authors or branch/path information, we want all code by all authors to be reviewed by our designated reviewer. You can see that Upsource suggests all the users in this project, so you can simply select one from the list.
   

   Figure 1: Configuring a trigger for creating reviews

6. Once this has been saved, you can see the details of the trigger:
   

   Figure 2: Viewing the trigger details

7. Next time anyone checks in code, a code review for this commit will be created automatically, to be reviewed by Reviewer:

Figure 3: The automatically created review

Notes:

By default, this creates a new review for every commit. You can optionally set the review to automatically track the branch it is on, which may be suitable if the team uses feature branches or a similar workflow.

Figure 4: You can optionally set the created review to track the branch

There are other options to allow commits to be bundled into a specific review, for example using issue IDs or code review IDs to automatically add a commit to a review.

Suitable for:

• May be used for gateway code review process, where either one individual usually approves changes to code, or there’s a set of reviewers who are all automatically added to all reviews. Not so suitable for this process if the author usually makes a series of commits but only wants them to be reviewed when the code is “finished”.
• May be used for design evolution reviews, in this workflow you may choose all commits to be reviewed by all (or most) of the team.

Vary the Automatically Assigned Reviewers

The previous automation is useful if you a) know which developers will be required as reviewers and optionally b) can assign these based on either branches or code paths. Most teams are more flexible around who they assign to reviews, particularly if they’re following an iterative approach or using code review for knowledge sharing. Upsource can assign randomly selected reviewers from a given pool of suitable developers.

In this example, we’re going to specify a pool of developers who can be randomly assigned to review any code from any developer.

Steps:

1. We’re going to enable a second automated workflow as well as the previous one. Firstly, we need to set up the automatic creation of code reviews, like we did in the last example. The only difference is that this time we do not assign reviewers (see Figure 1 – in this second example we do not add reviewers).
2.  While we’re on the Custom workflows section, we need to go to the “Assign review participants automatically” heading. Here, we click the Enable check box, then click the Add trigger button that appears.
   

   Figure 5 – Assign review participants automatically trigger

3. As usual, Upsource will suggest users for both the “Created by” and “Users” fields. We’re going to leave the “Created by” field blank, because we want this to apply to all developers. But you can imagine that if you wanted this workflow to apply to just the junior/new developers, you could add them to this box.
4. We’ll select a set of reviewers for all code reviews. This could be just the most senior/experienced people on the team, or could be all developers if you’re aiming for shared code ownership with your reviews.
5. Now, we have the option of either assigning everyone in this pool to be a reviewer, or getting Upsource to select a random subset of these reviewers for each review. We’re going to configure Upsource to pick one person randomly, so the reviewers don’t get overloaded with too many reviews, and so that everyone gets to see all parts of the code.
   

   Figure 6 – Configure Upsource to select a single reviewer randomly from the pool

6. Click the Save changes button.
7. Now, whenever anyone checks in code, a review will automatically be created, and a random reviewer assigned.
   

   Figure 7 – Review created and random reviewer assigned

Suitable for:

• Teams who want developers to break out of silos, encouraging them to review code outside their usual zone
• Knowledge sharing – by assigning random reviewers to each commit, the whole team can eventually get to see changes to the whole code base.
• Iterative design – this can be used to assign the whole team as reviewers or watchers to every commit, or it can be used to randomly select reviewers. This latter option may be a good way to reduce some of the noise when using code reviews for design evolution.

Automatically add commits to a review by Issue Number

Many teams already tag every commit with the ID of the bug/feature ticket in the issue tracker. If this is standard practice for your team, and you want to create a code review for every issue, then you can turn on the feature in Upsource that will do this for you.

Steps:

1. Follow steps 1-3 from the first example to navigate to the Custom workflows page.
2. Go to “Add revisions to a review automatically” and tick the checkbox. As before, click Add trigger to define the settings.
   

   Figure 8 – Create a trigger on Issue ID

3. Don’t forget to click Save changes.
4. You will still need to create a code review for the first commit with a comment containing a keyword in the right format (either a Jira or YouTrack issue ID, or a predefined issue ID format), unless this is combined with one of the other automated workflows. But subsequent commits with this issue ID will be added to the code review for the issue.
   

   Figure 9 – Commits with the same issue number are added to the same review automatically

Suitable for:

• Teams who do not work on feature branches but use issue numbers to group their commits.
• Teams who already tag all commits with issue numbers, or who would like to do this.
• Applies to all code review workflows.

Automatic integration with GitHub

This has been covered in previous blog posts. Upsource’s bi-directional integration with GitHub makes it easy to synchronise pull requests (reviews) and comments, so no matter whether a reviewer or author prefers to use the GitHub UI or Upsource, all the information is available in both tools.

Suitable for:

• Teams who use GitHub
• Open Source projects using GitHub where external contributors use the GitHub UI and internal team members have access to an Upsource instance.

Summary

Upsource has a number of options to help you automate your code review workflow. Just as there are many tools that automate the routine things to examine in your code (e.g. static analysis tools and automated tests) which should be used to minimise the load on your code authors and reviewers, there are ways to ease the creation of code reviews according to your workflow.

One of the options described above might suit your need, or more likely a combination of them will cover a vast majority of the cases where code reviews need to be created for your team, so you don’t even need to remember to create a review and assign people to it.

In the last two posts we talked a bit about how effective those approaches are for the author to learn and improve, because those two approaches focus on getting the code correct. The Design Evolution approach additionally mentions the value of the whole team seeing the evolution of the code.

This third approach for code reviews puts learning at the centre of the process — the purpose is not to check if the code is correct, but to share with other developers what changes have been made.

In this example, I’m assuming a slightly different approach to development. This team develops against master (or some other centralised development branch), and submits small self-contained commits that pass all the automated tests.

The Process

In this example we assume the team is not working on feature branches, but instead commits directly to some main development trunk. It’s important in these circumstances to have a CI system that runs all relevant checks, and that failures here are addressed with the highest priority

When I work on a fix or feature, as usual I’ll make changes in small, self-contained commits that pass all the tests. This should be the default way of working, of course, but when you’re committing to the main development branch this is so important, it ensures the whole team isn’t blocked on some breakage you introduced. Upsource integrates with your CI server so you can see at a glance the revisions that passed all the checks.

Upsource shows the result of the Continuous Integration build

In this code review process, I can either make each small change a tiny review in its own right, or I can batch them up into a single review when the feature is complete¹. The first approach has the advantage that the review will be small, but there will be many of them. The second approach means the review will probably be bigger, but gives me much more freedom to experiment and evolve my design during implementation.

In Upsource, I can either choose to add each revision to the review via the UI…

…or I can use the IntelliJ IDEA plugin to select which code review to add the commit to…

…or I can tag the commit with an issue number or code review ID to have it automatically added to a review.

Using this process, we’ll see groups of commits on the same branch belonging to different code reviews.

When the reviewer comes to look at the code, they have very different goals to either a gateway reviewer or a design evolution reviewer. Here, the reviewer’s goal is to read the code to: understand what functionality was introduced and see how the application code is evolving. Of course, the reviewer is still free to make comments and suggestions. If a reviewer doesn’t understand the code for any reason (poor naming, confusing design, missing tests to document features etc) this is an important point to raise — code is likely to be read more often that written, and if a team member cannot understand the code right now, when the current state of the code base and the context of the need for the fix or feature is fresh in their mind, imagine how much worse it will be in the future when all of this context is lost.

I can make changes based on these suggestions and also commit these straight into the development branch. I’ll get the reviewer to look over these changes too, to see if they address the original problem.

As always, I can guide the reviewer, and future code-readers, by leaving comments in Upsource to document the more obscure code, or make notes of possible future changes.

By having these comments in Upsource but not attached to a review, future readers of this code will be able to see what my intentions were.

Discussion

Here the main beneficiary of the code review is the reviewer not the code author. It’s a way to encourage ownership of the whole code base by the whole team, without everyone having to work on all sections of the code.

It also emphasises code readability. While enforcing standards or design approaches will help readability, these may be too heavy-weight for many teams. Peer-reviewing code can improve the readability by focusing on small but important things like naming and understandable tests.

This process does not act as a gateway. Code is already merged, it’s assumed to be working as all tests pass. Any suggested changes can be made later, and made directly to the development branch as well.

The reviewer can be junior or new, they’re not acting as the guardian of the code design. Many teams see this as a good way to get people up to speed on their whole code base with fairly minimal cost to the more experienced developers on the team.

Advantages

• No waiting for the review to finish before merging the changes into production-ready code
• Shared code ownership reduces the bus factor for the team
• Focus on code readability can improve the code’s maintainability
• Since the aim is to help all the team understand the code, reviewers can (and should) be developers of all levels, and junior or new developers’ comments are as valuable as those of more experienced team members — your code should be understandable to everyone.
• A reviewer is likely to spend far less time on this kind of review than on a strict-correctness review like a gateway review.
• There are likely to be far fewer iterations than a design evolution review, and probably fewer than a gateway review. This means not only will the review take less time, but also there’s likely to be less work-in-progress since code authors won’t be working on other things while also iterating over the code review.

Disadvantages

• If you do not have automated tests to catch important things (automated acceptance tests to “prove” the fix/features does what it should; automated regression tests; automated performance test for perf-critical systems; automated security tests etc) or dedicated departments (QA, performance, security etc) to own those areas, it’s possible that code that doesn’t work, or breaks something, could go into production, due to the focus on readability rather than strict correctness².
• Given the code is already merged and “working”, there may be a temptation to review the code “later”, so code reviews (and subsequent feedback) may happen when the context of the original fix are not so fresh in the author’s mind.

Anti-patterns

The anti-patterns for the previous two code reviews are things that block the code review finishing, and therefore block the code going into production. Since this approach assumes the code is going into production and the review is for learning and sharing, there’s nothing in the code review process that blocks the code going live.

Suitable for

• Teams with the highest level of trust that the developers only commit code that meets the team’s standards.
• Or code bases that don’t need strict control over the design and can tolerate multiple different programming approaches.
• Teams with shared code ownership
• Teams with all key requirements tested (either automated tests or a QA team)

¹ “Complete” meaning passes all appropriate tests, e.g. acceptance tests, and/or meets the acceptance criteria set in the issue
² Of course, these reviews are still more likely to catch problems than no reviews at all