This article is part of a series on reducing code review time. It helps if you have read these two first.

1. If code reviews take too long, do this first (gather data)
2. What to do about nitpick comments in PRs (eliminate them)

In the second article above, I define a nitpick comment as one that is just an opinion and not a defect, where a defect is something that doesn’t meet some objective criteria. A bug is an obvious example but so is a style-guide violation. A nitpick is a comment where the author and reviewer might disagree, but the issue isn’t objective, and there isn’t an obvious way to resolve the problem.

I recommend that you have almost none of these kinds of comments in a review, but there are a few cases where it’s ok.

1. During onboarding: You should set the expectation to your new hires that the first month or so of PRs might have excessive commenting as a way of training on culture of the team. To make this less onerous, early PRs should be very small. The more junior the new hire, the longer this might last.
2. If requested: Sometimes we are really do want to ask for opinions on code that isn’t ready for merge, but you want more eyes on it. In that case, authors should tag their PR as something that they would like more help on, and then any suggestions are fine. Authors that are doing new things in the codebase (and possibly setting new standards) should proactively request (nitpick) opinions.
3. As a precursor to setting a standard: if you have a strong opinion that you think should become a team standard, then starting the conversation in a PR comment might be warranted. I would recommend that you quickly move it to whatever process that your team uses to change their standards (e.g. a retro). Use the PR as an example.
4. If rare: If you think it won’t derail the PR and that the author would welcome it, then go ahead. A good example is pointing to an interesting technique that the author might not be aware of, but you think applies. This is more of an FYI than a request to change something.

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:

1. If code reviews take too long, do this first
2. What to do about nitpick comments in PRs

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:

1. Construct PRs to Make Reviewing Easy
2. PR Authors Have a lot of Control on PR Idle Time
3. 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:

1. 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.
2. 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.
3. If you can’t automate something, create a short checklist for the author to go through before posting the PR.
4. 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.

In “If code reviews take too long, do this first“, I said that the first step was to gather some data on your code reviews to identify pull requests that take longer than your goal time. One common reason I have seen is that code reviews take too long because there are excessive small, unimportant suggestions.

Not all small and unimportant suggestions are bad. The problem happens when a it’s just an opinion and the code isn’t objectively wrong. So, for example, a typo in a code comment is worth pointing out because the author can fix it without further questions, and the comment is improved. Another example is a clear violation of the style guide, like using a variable with underscore separators when your standard is camelcase. In these cases there is no argument that the original code is correct, and it’s trivial to fix and recheck.

But comments that are just an opinion like: “I don’t like this variable name” or “you could use map() instead of iterating the array” or “this function is too long” are a matter of opinion in most cases. It’s not as clear what to do about it and might end up causing some more conversation. The author’s second try might also not satisfy the reviewer. If this code was clearly wrong, that would be fine, but it’s not worth the extended time to fix a difference in opinion.

To address the problem of excessive nitpick suggestions, the team should adopt a standard that a PR comment should be pointing out a defect. Here is a short list of some obvious defects:

1. An off-by-one bug in a loop
2. The screen doesn’t match the specification
3. An error case is not checked
4. Typos in variables, function names, or comments
5. Using tabs when team standard says to use spaces (or vice versa)
6. Putting business logic in a view when the team requires an MVVM pattern
7. The code reimplements a function when it should use the one in our shared library

This is not an exhaustive list, but the pattern should be clear. A comment is pointing out a defect when it can compare the code to some agreed upon requirement or standard. It’s also clear what to do about it, and that it must be done.

So, if you find in your data that you have a lot of nitpick comments, gather them all together and go through each one and categorize them:

1. OK: If the comment is pointing out a clear defect against an agreed upon requirement, then that’s a good comment, and the problem is not the comment.
2. To document: If the comment is not a defect, but you think the comment is good, and there is a pattern of this kind of comment, then maybe this belongs in your coding standard guide such that it’s clear that the code is defective and what exactly to do about it. After this is documented, future comments of this nature are OK.
3. To eliminate: If the comment is not a defect and you don’t even think it should be a standard, then this comment should never have been made.

After doing this step, in the future, most comments should be OK, and sometimes there will be a comment where it’s unclear whether it should become a new standard or not an acceptable comment. Things like that can be hashed out using whatever process you have.

It could still be the case that there are too many comments and that is still the main reason that code reviews are taking too long. In the next post, I will explain what to do about the problem when you have too many good comments on your code reviews.