A copy-paste code review checklist, the order to review in, comment prefixes like Conventional Comments, PR size guidance, and how to record review decisions.
The KnowWhyy team · · 8 min read
Code review often goes one of two ways. Either a reviewer skims a 900-line diff, types "LGTM" and moves on, or they leave 40 comments about variable names while missing the design problem in the first file. Both waste time, and neither catches what matters.
A checklist helps, but only if it's short enough to use and ordered so the big questions come first. This post gives you a copy-paste checklist, the order to work through a pull request, how to phrase comments so authors know what's blocking, and how to keep the decisions made in review from vanishing when the PR is merged.
Before the checklist, agree on what "approve" means. Google's standard of code review is a useful default: favor approving a change once it definitely improves the overall health of the codebase, even if it isn't perfect.
Two consequences follow from that:
There is no perfect code, only better code. Don't hold a PR for days over polish. Leave the comment, mark it as optional, and approve.
Facts beat preferences. The style guide settles style questions. Design questions are settled by engineering principles and data. If the author shows that several approaches are equally valid, accept theirs.
Review in this order
The order matters more than the list. Google's guide to suggests three steps: take a broad view, look at the main part, then go through the rest.
Ask your first question in minutes.
Create a workspace, connect a tool and choose what gets indexed. The free plan includes GitHub, Slack and Notion.
1. Read the description first. Does this change make sense at all? If it shouldn't happen, say so right away and suggest what to do instead, before anyone spends more time. A PR with no description is a reasonable thing to send back. A pull request description template makes this step much faster.
2. Review the main part for design. Find the file or files with the most important logic and start there. If you see a major design problem, send that comment immediately, even before reviewing the rest. Much of the other code may change anyway.
3. Check functionality. Does it do what the author intended, and is that good for users? Think about edge cases and concurrency. For UI changes, try it or ask for a demo, since behavior is hard to judge from code alone.
4. Look for unnecessary complexity. Can the code be understood quickly? Google's guide asks reviewers to watch especially for over-engineering: code made more generic than it needs to be, or features nobody needs yet.
5. Check the tests. Are there tests in the same PR? Would they actually fail if the code broke? Tests are code too, and complexity in them still costs maintenance.
6. Read names and comments. Good names communicate without being long. Comments should explain why code exists, not restate what it does.
7. Check style last. Follow the style guide. Anything not in it is preference, so prefix it as a nit and don't block on it. Better still, let a formatter and linter handle style so humans don't have to.
8. Check docs. If the change affects how people build, test, use or release the code, are the README and other docs updated?
Reading the tests before the main code can also help. Google's guide notes it gives you an idea of what the change is supposed to do.
The checklist
Paste this into a team doc, your CONTRIBUTING.md or a saved reply.
### Context
- [ ] I understand what this PR does and why (description, linked issue)
- [ ] The change makes sense to do now, in this place
### Design
- [ ] The overall approach fits the rest of the system
- [ ] No simpler design would do the job
- [ ] Nothing is built for hypothetical future needs
### Correctness
- [ ] It does what the description says
- [ ] Edge cases: empty, null, large inputs, errors, retries
- [ ] Concurrency: races, deadlocks, ordering
- [ ] User-facing changes tried or demoed
### Safety
- [ ] Inputs validated; no secrets or personal data in code or logs
- [ ] Permissions and access checks are right
- [ ] Migrations and config changes are reversible, or the rollback is clear
- [ ] A specialist reviewed security, privacy, accessibility etc. if needed
### Tests
- [ ] New or changed behavior has tests
- [ ] Tests would fail if the code were broken
- [ ] Tests are readable and not more complex than needed
### Readability
- [ ] Names say what things are and do
- [ ] Comments explain why, not what
- [ ] Style follows the guide (nits marked as nits)
### Docs and history
- [ ] README and docs updated if behavior or setup changed
- [ ] Decisions made in review are captured in the PR description or code
Don't treat every box as mandatory for every PR. A one-line config fix doesn't need a concurrency review. The checklist is there so you don't forget a category, not so you can tick 25 boxes on a typo fix.
The "Safety" section is our addition rather than a quote from any one guide. Google's guide does say that if you're not qualified to review part of a change, such as privacy, security, concurrency, accessibility or internationalization, you should make sure someone who is gets added.
Keep pull requests small enough to review
A checklist can't save a review that's too big to read carefully. Google's guide to small changes is specific:
There's no hard rule, but 100 lines is usually a reasonable size and 1,000 lines is usually too large.
The number of files matters too: 200 lines in one file might be fine, but spread across 50 files it's usually too big.
Keep refactors in separate changes from features and bug fixes.
Reviewers may reject a change for being too large, and the usual fix is to split it into a series of smaller changes.
If you receive a PR too large to review soon, the guide's advice on review speed is to ask the author to split it. If that isn't possible, at least comment on the overall design so they can keep moving.
Respond quickly, without breaking your focus
The same speed guide sets a clear expectation: one business day is the maximum time to respond to a review request. It also says not to interrupt focused work to review. Wait for a natural break, like after lunch or a meeting.
Speed here means response time, not total time to merge. If you can't do a full review yet, say when you will, or suggest another reviewer. And when the remaining comments are minor, approve with comments rather than making the author wait another round. This matters most across time zones.
Comment etiquette
How you phrase a comment decides whether it gets acted on or argued about. Google's guide on writing review comments recommends:
Comment on the code, not the person. "This adds concurrency without a performance benefit, so a single thread would be simpler" lands better than "Why did you use threads here?"
Explain why. A comment that gives the reason teaches something and is easier to accept.
Point out problems; don't always solve them. Fixing the PR is the author's job. Sometimes direct code is more helpful, but often letting the author choose leads to a better result.
Praise good work. Say what you liked and why.
Label severity. Without labels, authors may treat every comment as mandatory.
Comment prefixes
Google's guide suggests prefixes like Nit:, Optional: and FYI:. Conventional Comments is a more complete standard with the format <label> [decorations]: <subject>. Its recommended labels include:
Label
Use it for
Blocking?
praise:
Something done well (be sincere)
No
nitpick:
Trivial, preference-based requests
No, by nature
suggestion:
A proposed improvement, with why
Say so with a decoration
issue:
A specific problem; pair with a suggestion
Usually
question:
A possible concern you're not sure about
Depends on the answer
todo:
A small, necessary change
Yes
thought:
An idea, not a request
No, by nature
chore:
A process step needed before acceptance
Yes
note:
Something to be aware of
No, always
Decorations such as (blocking), (non-blocking) and (if-minor) make intent explicit:
issue (blocking): This retries without a limit if the provider returns 429.
suggestion: Cap retries at 3 and back off, like PaymentClient does.
nitpick (non-blocking): `data` -> `invoiceRows` would read better here.
question: Does this run before or after the cache is warmed?
praise: Nice test for the concurrent signup case. That's exactly the bug we hit before.
Pick one scheme and write it down. The labels matter less than everyone agreeing what "blocking" means.
Start a review instead of adding single comments, so the author gets one notification instead of 20.
Suggest changes on specific lines so the author can apply them directly.
Resolve conversations once they're addressed, so open threads are easy to see.
Open an issue from feedback that's out of scope, linked back to the comment, instead of growing the PR.
Record the decisions made in review
Reviews are where a lot of small decisions get made: why the retry limit is 3, why the cache was left out, why an approach was rejected. Then the PR merges, and those decisions live in a collapsed thread nobody will find.
Google's guide is direct about this. Explanations that exist only in the review tool don't help future readers. If a reviewer asks what code does, the usual answer is to make the code clearer, or occasionally add a comment, rather than explain it in the thread. And if a disagreement gets settled in a call, the standard of code review asks you to record the outcome as a comment on the change.
A simple routine before merging:
Update the PR description with any decision made in review: "Kept retries at 3 because the provider rate-limits per card."
File follow-up issues for deferred work, and link them from the PR.
This is also how teams avoid relitigating the same decisions every few months. If the reason is written next to the code, the next reviewer can read it instead of reopening the debate.
Where KnowWhyy fits
Even with good habits, much of the reasoning behind your code is spread across PR threads, issues and Slack. KnowWhyy indexes the GitHub repositories, Slack channels and Notion pages you choose, including commits, pull requests and issues with their discussions (not the source code itself). Ask "why is the payment retry limit 3?" in the web app and the answer links to the PRs and threads it came from, or tells you the sources don't cover it. See GitHub history search, or start free.
The short version
Approve when a change clearly improves the codebase, even if it isn't perfect.
Review in order: description, main design, functionality, complexity, tests, names, style, docs. Send design problems first.
Keep PRs small. Google's guide calls 100 lines usually reasonable and 1,000 usually too large.
Respond within one business day, but don't break your own focus to do it.
Label comments (Conventional Comments or Nit:/Optional:/FYI:) so authors know what's blocking.
Before merging, move decisions from review threads into the PR description, code comments or an ADR.