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
- 1Is it solving the right problem? The most expensive comment to receive late, and the first one a good reviewer makes.
- 2Will it break something? Edge cases, concurrency, what happens on the error path.
- 3Can it be understood in a year? Naming, structure, the comment that explains why rather than what.
- 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 request | What actually happens |
|---|---|
| 50 lines | read properly, real comments |
| 500 lines | skimmed, a few comments about naming |
| 2000 lines | approved, 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.