News

How to Reduce Code Review Cycle Time

DanielGallagher
Partner

Code reviews are almost ubiquitous on our projects. We have worked with clients whose review processes provide significant value, and some whose processes are more hindrance than help. Our team meets weekly to learn new things and share best practices, we call them Software Discos. Here are our tips to reduce code review cycle from a recent disco.   

The Why

Before we get into reducing the time it takes to do code reviews, let’s quickly review why we do them in the first place. There are a lot of obvious benefits for doing code reviews. They can find and fix bugs before they are deployed and they are a good way to share knowledge of the codebase and teach methodologies. In some cases, they are also required by clients or government bodies like the FDA. 

However, there can be downsides to code reviews too. They can slow down the process. It can also be frustrating when feedback is more preference-based than requirement-based or when people use a code review to make a developer look bad. Some developers may feel that performing code reviews takes time away from “their development.” Though they are meant to catch bugs, a bad review could actually introduce bugs via bad advice. However, all of these can be avoided with a good process. 

We’ll take it as given here that you want or are required to do code reviews. We strongly encourage you to adopt a review process if you haven’t already! We want to ensure an effective code review process, not scare you away from having one at all.

Set Clear Expectations

Who is required to review code? 

We’ve found that the ideal is between 2-4 people to reduce code review cycle time. One person can cause a bottleneck while more than four people usually result in one of two extremes: either everybody leaves lots of comments or nobody leaves comments, assuming someone else will, the diffusion of responsibility in action.

• If you want the whole team to become familiar with the code, clearly separate roles for reviewers and people who have access to read the code without commenting or approving. 
• If you want to train junior reviewers, do a pair review with the author or an experienced reviewer.

Some organizations have code “owners” or “custodians” who are responsible for reviewing every change in a particular section of code. We prefer to work without strict code ownership, where everyone on the team is empowered to make fixes throughout the code base. But, in large codebases, owners can be a last line of defense against bugs creeping into the code. We strongly recommend that you use owner groups if at all possible. That lets authors know who to add to the review, but prevents any single person being a bottleneck. If you do have owners, you need to ensure their domain of responsibility is small enough that they can review everything without holding up other developers.  

When should code reviews be completed? 

One of the best ways to increase the effectiveness and efficiency of code reviews is to minimize the time between writing the code and merging it to the primary branch. If you wait too long, you will forget why you did what you did. As a result, you may accept advice that goes against your original plan. 

Ideally, review code the same day to reduce code review cycle time. If that’s not possible, make sure everyone on the project knows how long reviews will take so authors can plan for the delay by having multiple work streams going at once. Just in case the review takes a while, get into the habit of adding a comment to anything that is not standard practice. This will communicate your thinking to the reviewer and help you remember as well.

What code should be reviewed?

Small patch size is extremely correlated to reduced code review cycle time. As a result, reviews can be done by a small team and within a short timeframe – achieving the who and when tips above. 

Influence of the Patch Size (LOC) over Duration (days), Participation (%), Comment Density (comments per 100 LOC) and Comment Density by Reviewer (comments per 100 LOC per active reviewer). The y-axes representing Comment Density and Comment Density by Reviewer are in a logarithmic scale.

From “Investigating the effectiveness of peer code review in distributed software development based on objective and subjective data – Journal of Software Engineering Research and Development“

Get Fluent in Git 

Getting comfortable switching between branches locally and saving work for later is helpful both as an author (so you can switch between your current work in progress and a branch that you need to update for reviews) and as a reviewer (if you want to check out code locally for testing). Check out the book Pro Git for more information.

Some common tasks to juggle code are:

• git stash especially if I’m checking out someone else’s code for review and will get back to my work shortly
• git stash pop to undo a stash
• git commit -m "wip" especially if I’m saving my work to switch back to an old branch for PR updates
• git reset HEAD~ to undo the previous command without losing work
• git rebase -i to help keep commit history clean, so I don’t have “Addressing PR comments” commits
  • Some code review tools (such as Gerrit) essentially require you to get good at editing multiple commits on a branch.
  • Other review tools may get confused if you rewrite history during a review. In that case, it may be better to have “Addressing PR comments” commits during a review. We still would recommend doing a final pass to clean up the branch prior to merging to the primary branch. If your team uses squash merges, this may not be an issue.
• git add -p to make multiple commits out of my working changes, and git stash -k to make sure those intermediate commits build and/or work.

Make Testing Easy

Your PR should include automated tests that run alongside the new production code. Tests let reviewers examine the code from an outsider’s perspective in addition to the author’s perspective. It makes it easy to see the intended API, and whether or not it will be ergonomic for callers.

For firmware projects (or software projects that involve a hardware component), making testing easy can be more challenging, especially if hardware availability is limited. This is a priority if code reviewers are tasked with running the code to verify it. It becomes less important as the code review process is easier and faster, since bugs can be fixed more easily than with an arduous code review process.

Make Code “Perfect” 

One way to to reduce code review cycle time is to ensure reviewers have nothing to say. Of course that’s not a practical goal but, in general, lessons can be learned from reviewers past comments and, over time, those comments can be addressed before the code is put up for review.

This section includes some tips from Curtis Einsmann via his Medium article:

• Read the existing code. Make sure that your code fits the patterns of the existing code. This goes beyond the style guidelines (which should be automated as much as possible), and is more about making sure that you are doing things the way the existing code does them. Look for code that you can reuse, and either reuse it or refactor the code so that it is reusable, then reuse it.
• Plan your implementation. Especially if there is some refactoring involved, it can be helpful to write down the steps you plan to take, and where you expect your code to end up (classes, functions, etc). Plans change, but having a plan can make the implementation more review-friendly if you can make your commits tell the same story as your plan.
• Refactor for readability. First, write the code any way you want, then refactor to ensure it is all readable. Micro-commits are helpful here. I’ll often have a series of commits where I’ve made (for example) 5 somewhat related commits and a number of cleanup commits. Before making the PR, I’ll do a git rebase -i and change from