The PR review checklist I actually use
The PR review checklist I actually use
After a few thousand reviews: scope, read the test first, look for the thing that is not there, names, test altitude, and "could I debug this at 2am?" — plus what I never comment on.

On this page
I have reviewed a few thousand pull requests. Most of what makes review valuable is not catching bugs — it is catching the things that will become bugs, or that will make the next person slower. Here is the checklist I run, roughly in order, and why each item earns its place.
1. Does the PR do one thing?#
Before I read a line, I check the scope. A PR that "fixes the bug and also refactors the service and also renames three files" is three PRs wearing a trenchcoat. I ask for the split, every time. Not to be pedantic — a focused PR is reviewable, revertable, and bisectable. A mixed PR is none of those.
2. Read the test first#
I read the test before the implementation. The test tells me what the author thinks the code does, in executable form. If I cannot understand the feature from the test, the test is too coupled to implementation, or the feature is underspecified. Either way I have learned something before reading the real code.
3. Look for the thing that is not there#
The hardest review skill is noticing absence. The error path that is not handled. The loading state that is not rendered. The empty-list case. The second click. Reviewers are good at judging the code on screen and bad at noticing the code that should be on screen and is not. I keep an explicit mental list: error, empty, loading, race, off-by-one.
4. Names#
I push hard on names, and I make no apology for it. A function named handleData tells me nothing; a function named mergeDraftIntoPublished tells me the whole story. Names are the densest documentation in the codebase and the only docs guaranteed to stay in sync with the code. A rename costs the author thirty seconds and saves the next reader thirty minutes.
5. Is this tested at the right level?#
Not "is there a test" but "is the test at the right altitude." Business logic deserves a unit test. A wiring change deserves an integration test. A user flow deserves an e2e. A getter does not deserve a test at all. I have learned to flag both under-testing and over-testing — a hundred tests on trivial field assignments is noise that slows every future change.
6. Would I be able to debug this at 2am?#
The final pass. If this code breaks in production six months from now, and I am the one paged, can I find the problem? Is there a log at the boundary? Does the error carry enough context? Is the control flow linear enough to read under stress? Code is written once and debugged many times, often by someone tired.
What I do not comment on#
Formatting. Indentation. Quote style. Import order. All of it is handled by Prettier and the linter, automatically, and any human comment about it is wasted attention. If we are arguing about whitespace in a review, the tooling has failed, and the fix is tooling, not a comment. I spend my review budget on the things a machine cannot judge: scope, names, absence, and whether the code will be kind to the next person.