6  Handling pushback

Sometimes an author will push back on a code review. Either they will disagree with your suggestion or they will argue that you are being too strict in general.

6.1 Who is right?

When an author disagrees with your suggestion, first take a moment to consider if they are correct. Often, they are closer to the code than you are, and so they may have more insights about that particular section. Does their argument make sense? Does it make sense from a code health perspective? If so, let them know that they are right and let the issue drop.

However, authors are not always right. In this case, the reviewer should further explain why they believe that their suggestion is correct. A good explanation demonstrates both an understanding of the author’s reply, and additional information about why the change is being requested. Good explanations are additionally reinforced by facts or external official resources like the design or style guide.

In particular, when the reviewer believes their suggestion will avoid a drop in code health, then they should continue to advocate for the change if they believe the resulting code quality improvement justifies the additional work requested. On the other hand, if the PR already improves the overall health of the package, it isn’t worth arguing every little stylistic point. Code improvements often happen in small steps.

Sometimes it takes a few rounds of explaining a suggestion before it really sinks in. Just make sure to always stay polite and let the author know that you hear what they’re saying, you just don’t agree.

6.2 Upsetting authors

Reviewers sometimes believe that the author will be upset if the reviewer insists on an improvement. Sometimes authors do become upset, but it is usually brief and they become grateful later that you helped them improve the quality of their code. Usually, if you are polite in your comments, authors actually don’t become upset at all, and the worry is just in the reviewer’s mind.

6.3 Cleaning it up later

A common source of push back is that authors (understandably) want to get things done. They don’t want to go through another round of review just to get this PR in. So they say they will clean something up in a later PR, and thus you should approve this PR now. Some developers are very good about this, and will immediately write a follow-up PR that fixes the issue. However, experience shows that as more time passes after an author writes the original PR, the less likely this clean up is to happen. In fact, usually unless the author does the clean up immediately after the present PR, it never happens. This isn’t because the author is irresponsible, but because they have a lot of work to do and the cleanup gets lost or forgotten in the press of other work. Thus, it is usually best to insist that the author clean up their PR now, before the code is in the package and “done.”

Note that PR cleanup is different from the case where a PR exposes a bug that is technically unrelated to that PR. In the spirit of keeping PRs small and focused, in those cases it is best to open an issue with a reproducible example demonstrating the issue, so that it is tracked and can be addressed after merging the current PR.

6.4 Resolving conflicts

In any conflict on a code review, the first step should always be for the author and reviewer to try to come to consensus, based on the contents of this guide.

When coming to consensus becomes especially difficult, it can help to have a face-to-face meeting or a video conference, instead of just trying to resolve the conflict through code review comments. (If you do this, though, make sure to record the results of the discussion as a comment on the PR, for future readers.)

If that doesn’t resolve the situation, the next step is to escalate. Often the escalation path leads to having a third team member offer their own review of the review comments, or having a broader team discussion for particularly complicated issues. Don’t let a PR sit around because the author and the reviewer can’t come to an agreement.

Remember to respect your colleagues even when you disagree. You both want to generate the best results possible, and just happen to have different beliefs about how to get there.