Review Someone Else's Code
Review a change you did not write without quietly rewriting it in your own style.
When to reach for it
When you open a pull request, or a pile of agent output, and your first instinct is to restructure it before you have found a single defect.
What changes
- Comments are labelled blocking, worth considering, or preference, and only the first kind holds up the merge.
- Style opinions are either dropped or turned into a lint rule, instead of being relitigated on every change.
- Each blocking comment names the input and the expected result, so the author can reproduce the problem instead of guessing what you meant.
- You state what you read and what you did not, so an approval stops silently meaning “I skimmed the first file”.
- Where you lack context you ask a question instead of proposing a rewrite, which is how the reviewer's misreading gets caught rather than merged.
Pairs with
- Ask One QuestionWhen you are blocked, work out the single question that unblocks the most — and ask that one, well.
- Disagree With the BriefPush back on a requirement that will not survive contact with reality — with a specific failure case and a way forward.
- Estimate HonestlyGive a range, name the assumptions holding it up, and say what it would take to make it narrower.
Review Someone Else's Code
Your job is to find what is wrong, not to make it look like something you would have written.
1. Read the intent before the code
What problem is this solving, under what constraint, in what environment? If none of that is written down, ask before commenting. Half of all review arguments are two people solving different problems at each other.
2. First pass: correctness only
No style yet. Look for the things that hurt after merge:
- Input at the boundary that is not the shape the code assumes
- Error paths: swallowed, logged and continued, or genuinely handled
- Anything that can run twice — retries, double submits, replays
- Concurrency: shared state, ordering, transaction scope
- Data loss, and operations that cannot be undone
- Authorisation, secrets, and what ends up in the logs
- Resources opened and never closed
3. Second pass: does it fit here
Compare it to the code around it, not to the code you would have written. Different is not wrong. Inconsistent with its neighbours is worth a comment; unlike your habits is not.
4. Label every comment
Three kinds, and say which each one is:
- Blocking — must change before merge, with the reason
- Worth considering — a real improvement, author decides
- Preference — your taste, no action expected
If everything is blocking, nothing is.
5. Make blocking comments reproducible
Give the input, the expected result and the actual one. "This breaks when the list is empty" can be checked in a minute. "Are you sure about this?" starts a conversation and resolves nothing.
6. Say what you did not check
"I read the API layer, not the migration" is honest and useful. An unqualified approval claims more than you did, and it is the claim people remember afterwards.
7. Decide
Approve, or block with one sentence naming what must change. Do not leave fourteen comments and no verdict; that is not a review, it is weather.
Rules
- Do not rewrite the change in the review. Suggest the smallest edit that fixes the defect you actually found.
- Do not block on anything a formatter or linter could settle. Add the rule, or let it go, but do not spend a human on it twice.
- Do not review only the diff when the risk lives in what the diff assumes. Open the callers.
- Do not send a review made entirely of questions. Say what you think.
- Do not hold a change hostage to a refactor of code it did not touch.