Part 1 · 2 chapters · ~12 min
Reviewing a PR: An Order of Reading
Reading the description first, then tests, then the core change, then the wiring; running the change for UI and risky paths; a checklist for money, data, security and concurrency; reviewing large PRs; and time-boxing reviews.
3
Six steps
Reading a diff top to bottom in file order is the least effective way to review it.
AN ORDER FOR READING A PR
context first, then the heart of the change, then everything else
swipe the figure sideways, or tap expand for full screen
1/6
description
Read why the change exists and how it was tested. If you cannot tell from the description, ask before reading code; it will save both of you time.
why, what, how it was testedno description: ask first
4
A checklist for risky changes
| area | questions |
|---|---|
| money | idempotent? integer minor units? balanced postings? what happens on timeout? |
| data | migration safe for old and new code? locks taken? backfill batched? reversible? |
| security | authorisation checked server-side for this object? input validated? secrets kept out of logs? |
| concurrency | races between requests? retries? ordering assumptions? |
| failure | what if the dependency is slow or down? is there a timeout and a designed state? |
| observability | will we know if this breaks in production? logs, metrics, an alert? |
For a PR over about 400 lines, review in sessions of under an hour; research on inspections found defect detection drops sharply in long sessions. Or ask for it to be split (part 4).