Short answer: before you approve, make sure you know why the PR exists and that CI is green. Then check that the change does what it says and nothing else, that the risky parts (auth, data, migrations, public APIs) got a proper look, and that the tests would fail if the code were wrong. Anything else is either a linter's job or a matter of taste.
TL;DR
- Read the description and the linked issue before the diff.
- Let CI handle formatting. Save your attention for the rest.
- Review in passes: shape first, then logic, then edge cases.
- Use GitHub's own tools: Viewed checkboxes, hide whitespace, suggested changes, batched reviews.
- Say clearly what's blocking and what's a nit.
- Keep the queue short, or the checklist won't matter.
Why bother with a checklist at all?
Because most of us do reviews tired. You open a PR between two other things, skim the diff, see nothing on fire and click Approve. A checklist won't make you smarter. It does stop you skipping the boring questions, and those are the ones that catch real bugs.
The catch is that most checklists are too long. Forty items is a compliance form, and nobody reads compliance forms. The one below is short on purpose. If a check can be automated, it's not on the list. For the wider process around it (who reviews what, when, how re-requests work), see the PR review workflow guide.
Before you open the diff
Read the why
Start with the PR description and the linked issue. If there isn't one, that's your first comment. You can't tell whether code is correct if you don't know what it's meant to do. Google's code review guide puts design and functionality ahead of everything else for exactly this reason.
If your team keeps opening PRs with empty descriptions, add a pull request template. Put a .github/pull_request_template.md file in the repo and GitHub pre-fills every new PR description with it (GitHub docs). Something like this is enough:
## What and why
## How to test
## Risky bits / anything you want a second look at
The last heading is the useful one. The author knows where the bodies are buried, so ask.
Check the size
Look at the diff stats before you read a line. If it's 2,000 lines across 40 files, the honest review is "can we split this?". SmartBear's write-up of a large code review study at Cisco found that reviewers catch far fewer defects once a session goes past roughly 200-400 lines (SmartBear). You're not the exception. Nobody is.
Is CI green?
If checks are failing, don't review yet. Leave a quick comment and move on. Reviewing code that's about to change anyway wastes two afternoons, yours and theirs. And if formatting, lint and type checks aren't in CI, fix that once instead of commenting on semicolons forever.
The checklist itself
Here's the version I'd pin. Copy it somewhere you'll see it.
| Area | What to ask |
|---|---|
| Purpose | Does the change do what the description says? Is anything in the diff unrelated? |
| Design | Is this the right place for this code? Would the next person find it? |
| Logic | Off-by-ones, null and empty cases, error paths, retries, timeouts |
| Data | Can the migration be rolled back? Is the backfill safe on big tables? Anything destructive? |
| Security | Input validation, authorization checks, secrets, anything logged that shouldn't be |
| Tests | Would these tests fail if the code were wrong? Are the edge cases covered? |
| Public surface | API, config, CLI flags, schema: does anything break for someone else? |
| Readability | Names, comments that explain why, dead code |
| Ops | Feature flag? Logs or metrics to show it works in prod? A way to roll back? |
Not every row applies to every PR. A copy change doesn't need a rollback plan, and that's fine. Glance at each row and decide "n/a" on purpose, rather than never thinking about it.
The test question gets its own line
"Has tests" and "is tested" aren't the same thing. A test that mocks the thing under test, or checks that a function was called instead of what it returned, will pass forever. Quick sanity check: pick one line of the actual change and imagine breaking it. Does any test go red? If you're not sure, check out the branch and try it:
gh pr checkout 1234
# break something on purpose, then
npm test
It takes a couple of minutes and catches more than you'd expect.
How to review on GitHub without losing your place
GitHub has decent tools for this, and most of us use about half of them.
- Review in passes. First pass: skim every file to get the shape. Second: logic. Third: edge cases and tests. Try all three at once and you'll miss the interesting bit in file 14.
- Tick Viewed. Every file in the Files changed tab has a Viewed checkbox. It collapses the file, and if the author pushes changes to it, GitHub flags it as changed since you viewed it. That's your progress bar.
- Hide whitespace. It's in the diff settings, or add
?w=1to the URL. Re-indented blocks stop looking like rewrites. - Only look at what's new. On a second round, use the commits dropdown in Files changed to see changes since your last review instead of re-reading everything.
- Batch your comments. Click Start a review instead of Add single comment. The author gets one notification instead of twelve, and you can delete the comment you wrote before you understood the code.
- Suggest, don't describe. For small fixes, write a suggested change. The author can commit it straight from the PR:
```suggestion
const timeoutMs = config.timeoutMs ?? 5000;
```
GitHub's docs on reviewing proposed changes cover all of these, including multi-line suggestions and filtering by file type.
Approve, comment or request changes?
When you submit a review you get three options. Pick on purpose.
- Approve means "I'd be fine if this shipped as is." Nits are allowed. Mark them as nits.
- Request changes means "this shouldn't merge yet." On a branch that requires reviews, it blocks the merge until you approve or someone dismisses your review. If that setup is new to you, here's how required reviews work.
- Comment is for questions, or for "I only looked at part of this."
Say how serious each comment is in the comment itself. A prefix is enough:
blocking: this drops the transaction if the second write fails
question: why a new client here instead of the shared one?
nit: could be a const
Conventional Comments turns the same idea into a spec, if your team likes a spec for everything.
The rule that matters: don't block on a nit. Nobody remembers who approved a PR with a slightly clumsy variable name. Everyone remembers the PR that sat for four days over one. And if you can never tell whether a reviewer has actually looked yet, here's what reviewer status on GitHub means.
The checklist doesn't help if PRs sit for days
Honestly, the bigger problem on most teams isn't review quality, it's how long reviews take. A careful review three days late is worse than a decent one three hours in. By then the author has moved on, rebased twice and forgotten the thing you're asking about.
So the last item isn't a check, it's a habit: know what's waiting for you. GitHub search can tell you if you bookmark the right queries:
is:pr is:open review-requested:@me
is:pr is:open user-review-requested:@me
is:pr is:open reviewed-by:@me -author:@me
is:pr is:open author:@me review:changes_requested
The first query includes requests sent to your teams, and the second only shows the ones that name you. The third finds PRs you've reviewed that are still open, which is where "I'll approve once they fix that" goes to die. The fourth is your own work waiting on you. The full list of qualifiers is in GitHub's search docs.
Bookmarks work, but each one is another tab, and we've all ended up with fourteen of those (there's a whole post on that). That's the problem PRFlow is built for. It sits in Chrome's side panel and lists open PRs across your repos with their status (pending, changes requested), labels, comment counts and diff size. There are filters for needs my review, created by me, and reviewed but not merged. Since the diff size is right there, you can do the size check from earlier before you even click in.
What it doesn't do: it won't review the code for you and it won't enforce this checklist. The line-by-line work still happens in GitHub's Files changed view. PRFlow is the queue, not the reviewer. If you're curious about tools that try to do the reviewing part, there's a separate piece on AI pull request review.
Making it stick on a team
A checklist nobody sees is a checklist nobody uses. A few cheap ways to keep it in front of people:
- Put the author's questions in the PR template, so "how to test" and "risky bits" are answered before you have to ask.
- Use CODEOWNERS so the person who knows the migrations sees every migration. A line like
/db/migrations/ @your-org/datain.github/CODEOWNERSdoes it. - Save the reviewer table as a saved reply (Settings, then Saved replies) so it's one click away in any comment box.
- Agree on what Approve means. Say it out loud, once. Half of review friction is people meaning different things by the same green button.