I estimate I've been on the receiving end of roughly four thousand pull requests over the course of my career. I have written, at a guess, something similar on the way out. A surprising fraction of those reviews did not work — they shipped bad code, or they created friction so severe that the author resented the reviewer for months afterwards. Neither outcome is what you want.

Ten rules I wish every reviewer I worked with had known. They are not novel. They are all obvious after you've broken them once.

1. Review the diff, not the author

Comments attack code. They do not attack people. The moment a review comment starts with "you" it is worth re-reading — usually it can be rephrased as a comment about the code without losing information. "You forgot to handle the null case" is a judgement on a person. "This branch is missing the null case" is a judgement on a line of code. The second is better even if the meaning is identical.

2. Read the whole PR before commenting on anything

It is painfully common to fire off a comment on line 12, then on line 45, then on line 98, and realise by line 300 that the first three comments were wrong because the author addressed them structurally further down. Read the whole change first. Then comment. This also helps you find the one or two comments that actually matter, rather than spraying a hundred small ones.

3. State whether something is blocking

Every comment should be tagged, implicitly or explicitly, as either "this blocks merge" or "take it or leave it". Without the tag, the author has to guess. Most of the frustration in code review — "why is this person dragging out my merge?" — is authors assuming all comments are blocking when the reviewer thought they were suggestions.

I use the convention nit: for non-blocking comments, q: for a question, and nothing for blockers. If you prefer a different set, fine, just pick one and tell your team.

4. Ask questions before you make assertions

Nine times out of ten, when I find myself writing "this is wrong, should be X", I'd be better off asking "why did you go this way instead of X?". The author often has a perfectly good reason. If they don't, they'll see the alternative themselves. If they do, I've just learned something I would otherwise have missed.

5. The author is responsible for the change, not for pleasing you

A comment that reads "I'd have done it differently" is not a reason to change the code. It's a reason to have a discussion if the difference matters, and to let it go if it doesn't. The bar for blocking someone else's change is not "I wouldn't have written it this way". It is "this will cause a problem."

6. Approve small PRs fast

If a change is under fifty lines and obviously correct, approve it. Today. Within the hour if you can. Fast approvals on small changes shape the culture of the team — they make engineers comfortable shipping in small increments, which is the single biggest quality lever I know of.

The inverse is also true. If an author consistently ships 800-line PRs, the fix is not more thorough review. The fix is a conversation with the author about splitting work up.

7. Big PRs deserve a phone call, not a 40-comment review

A PR that touches thirty files and 1,500 lines cannot be reviewed well asynchronously. The right response is to book a call, walk through it with the author for thirty minutes, and either request it be split or leave a short summary of the two or three things that matter. Leaving forty comments on a big PR is an act of revenge, not engineering.

8. Tests count as much as the code

Review the tests with the same care you review the implementation. In particular, check whether a test is actually testing what it claims to test. I have, embarrassingly, seen tests shipped where the assertion was commented out and the reviewer missed it because they had focused all their attention on the main change.

9. Praise what is genuinely good

Reviews skew negative because reviewers are trained to find problems. The occasional "this refactor is much cleaner than what was there before" comment costs nothing and changes the colour of the whole conversation. Do it when you mean it. Don't do it when you don't — performative praise is worse than silence.

10. Timestamp your own code after twenty-four hours

A rule for authors, not reviewers. Before you ask someone else to review your change, look at it yourself twenty-four hours after you finished writing it. You will catch something the reviewer would otherwise have caught. It is cheaper to find it yourself.

A note on junior reviewers

Junior engineers reviewing senior engineers' code is one of the healthiest habits a team can build. The junior learns a great deal; the senior is forced to write code that is explicable to someone without all their context; and the review itself is often better because the junior asks the obvious questions the senior has stopped asking. Encourage it. Don't skip it.

— Nivaan