If you want mandatory code review to really suck, make it slow.
Power games aside, one of the most frustrating aspects of code review is how long it takes and how many context switches it involves. There are two main things that contribute to this:
- how long it takes for the reviewers to react to new and updated pull requests (PRs).
- how many rounds of review are needed
In my experience in a 2-5 person team, if everybody is willing to dedicate some time for reviews every day, the wait for reviews is never too long. If I can get reviews for morning PRs in the afternoon and for the afternoon PRs in the next morning, I’m pretty happy already.
If your team members are not willing to dedicate this time, usually it’s because they either do not see code review as valuable – possibly because the management does not see it as valuable! – or they don’t know how to do it. You’ll have to solve those issues before you can improve the review velocity.
When reviewing, I use a three-level system of approvals:
- Not approved; changes are needed and the PR needs to be re-reviewed after the changes.
- Approved conditionally: the author needs to do some changes, but they can merge the PR afterwards without another review round. For example, if I point out something minor like a typo, I trust that the author can fix it themself.
- Full approval. The PR can be merged as-is.
Ideally a most of your PRs would be approved with a single review round with some needing another round. There are a couple of things that help:
Keep PRs small. A shorter PR is easier to review than a longer one.
Give actionable feedback. The second round of review will go more smoothly if the author knows exactly what changes the reviewer wants to see to approve the PR.
Automate what you can. If you care about consistent syntax formatting, use a linter or an autoformatter to enforce the format. Then you can ignore it entirely in the review process.
Figure out what to do if consensus cannot be reached. Sometimes the author and the reviewer can’t agree. My personal rule is that if there’s no obvious authority, the author makes the decision. You want to be mindful of power dynamics, though.
Figure out what to do if no-one wants to approve the PR. For example, reviewers may hesitate to approve the PR when they are not very familiar with the codebase. My personal rule is that the PR should be approved unless the reviews can give constructive feedback. The PR won’t get any better by sitting on it.
If many of your PRs seem to need three or more rounds of review, you might want to change some other part of the process than code review:
- Having a design discussion before implementing anything helps to ensure that nobody has fundamental disagreement with the approach taken during the review process.
- Having clear coding standards helps to avoid unnecessary conflict in the review phase.
- Pair coding is more efficient than multiple rounds of review. If it looks like a PR needs a lot of changes, consider pair coding the improvements with the author. Then there are at least two people who think it’s an adequate solution.
See also my other posts about code review.