This post is part of a series on reducing the time it takes for a PR to go through code review. Here are the first two articles:
In step 1, we gathered data, and in step 2, we took steps to eliminate nitpick comments. But, we could still have slow code reviews because there is a lot of discussion about the pull request that needs to happen before the code is merged.
Some of this is fine. There will always be outlier pull requests that merit careful attention. However, if this is the norm, then it’s worth taking steps to reduce the code review feedback loop because (according to DevEx) long feedback loops are one of the drivers of low developer productivity.
When you have a lot of good commenting on a PR it might be a sign that the author isn’t checking their own code enough before they create the pull request. Spending an hour or so making sure that a pull request doesn’t have obvious problems saves the time of the reviewer but also reduces the feedback loop, which could go over several days.
On my team at Trello, almost all PRs were approved within a day because we had a culture of making sure that PRs were easy to review. I have documented some of our practices here:
- Construct PRs to Make Reviewing Easy
- PR Authors Have a lot of Control on PR Idle Time
- Pull Requests for One
To construct a PR that is easy to review, the code must be correct and also easy to know that it’s correct. Here are some ways to reduce the time it takes for reviewers to approve PRs:
- Use automatic linters and code formatters that make it impossible to PR code that doesn’t meet your coding standards. The reviewer doesn’t need to check for style problems as they are impossible.
- Go beyond simple automations. Automated review tools exist for accessibility, security, code coverage, static analysis, etc. These should establish a baseline to assist the reviewer and not replace them. Reviewers should not look at PRs until they pass the automated checking.
- If you can’t automate something, create a short checklist for the author to go through before posting the PR.
- If it’s clear that a PR has lots of problems, a lead should have a 1:1 with the author to train them on self-checking and pair program a better PR with them. This avoids back-and-forth that might take days. The goal should be to get the PR into a state that the lead would approve.
When PRs are slowed down because of excessive back-and-forth between the author and reviewer, but all of that discussion was necessary, that is an indication that not enough work is being done by the author to make the code obviously correct. Don’t discourage the commenting or short-cut the review in this case.