Reviewing code I disagree with ⤶

I disagree with code in pull requests every week. Most of those disagreements are not worth a comment, let alone a block. Figuring out which is which is a skill I had to learn the slow way, by losing arguments I should not have started in the first place.

What I do now, before I type anything, is sort the disagreement. Roughly there are three places it can land.

Blocking

The change is wrong, or it makes the codebase worse in a way that will compound. A few cases that earn a block from me:

• The change is incorrect and will fail in production for a case I can name.
• It introduces a security or data-integrity issue.
• It locks in a decision that will be expensive to reverse: a public API, a database schema, a cross-service contract.
• It violates a convention the team has explicitly agreed on, not one I assume they would.

A block needs a written reason, and where possible the smallest concrete suggestion that would unblock it. “I don’t like this” is not a block. “This breaks for users in timezone X, here is the failing case” is.

Comment and approve

I disagree, but reasonable people could land on either side. The author has thought about it and made a defensible choice. I leave a comment explaining what I would have done, mark it non-blocking, and approve.

This is the category I had to grow into. Early on I treated every disagreement as worth a block, because every disagreement felt like a chance to be right. The cost of doing this is invisible at first and very visible later. Review queues stop moving. The senior people on the team stop reviewing because nothing they touch ever lands.

The default here is approve. The comment is meant as help, not as homework.

Just let it go

Sometimes I disagree because I have not seen the style before, or because I have a habit the author happens not to share. No comment. Approve.

The test I use is: would past-me have written it the way present-me would write it now? If the answer is no, my preference has shifted with experience and is now taste, not judgment. The author does not owe me my latest opinion.

The order matters more than the categories

The mistake I see most often, in myself too, is people writing the comment first and then deciding which category it belongs to. By the time you get there the comment already exists, the author has already read it, and the question changes from “do I write it” to “do I withdraw it.” Those are not the same question. Sort first, type second.

Sometimes the disagreement actually is technical, and a PR is the wrong place to have it. Move it to a thread in the team’s chat, or an RFC, or a 30-minute call. PR comments are not good at decision-making, they are slow and public and they pile up out of order. The author should not have to defend a design choice through review comments while CI is failing.

A reviewer who blocks often is a reviewer the team learns to route around. Depth of comments is not really what makes a reviewer trusted. What does is the ratio of approvals to blocks, and how fast the queue moves when they are on it.