Skip to content

step 5 / 32 · 5 min

What a reviewer is actually looking for

Your first pull request comes back with fourteen comments. It feels personal. It is not, and knowing what a reviewer is scanning for makes it much less mysterious.

The order they look in

  1. 1Is it solving the right problem? The most expensive comment to receive late, and the first one a good reviewer makes.
  2. 2Will it break something? Edge cases, concurrency, what happens on the error path.
  3. 3Can it be understood in a year? Naming, structure, the comment that explains why rather than what.
  4. 4Style. Last, and mostly the linter's job.

A review that opens with style comments and never reaches point one is a bad review, and worth noticing when you are the reviewer rather than the author.

Size decides everything

Pull requestWhat actually happens
50 linesread properly, real comments
500 linesskimmed, a few comments about naming
2000 linesapproved, unread

That last row is not cynicism, it is what happens. A large pull request buys an approval, not a review. If you want your work checked, make it small enough to check.

Receiving one

The comments are about the code. Answer the question, change it or explain why not, and do not apologise. "Good catch, fixed" and "I did it this way because the other order deadlocks" are both complete answers.

Giving one

Ask rather than instruct: "what happens if this list is empty?" invites a check, while "this will crash on empty input" invites a defence and is sometimes wrong.

And say what is good. A review that only ever finds faults trains people to fear it, and a feared review gets smaller and less honest.