A good code review does two jobs: it keeps the codebase healthy, and it spreads knowledge across the team. It isn’t a gate where reviewers prove how smart they are, and it isn’t a hunt for perfect code. Google’s public guide to the standard of code review puts it plainly: there’s no perfect code, only better code, so a reviewer should approve a change once it clearly improves the overall health of the codebase, even if it isn’t exactly how they would have written it.
This guide covers both sides. First, how to prepare a pull request that’s easy to review. Then, how to review one well, with a checklist and examples of comments that help and comments that don’t.
For authors: make your change easy to review
Keep it small
A reviewer facing a huge diff tends to skim it. A reviewer facing a short one actually reads it. Google’s guidance on small changes says a change should do one self-contained thing, and that about 100 lines is usually a reasonable size while 1,000 lines is usually too large, with the reviewer’s judgment as the final word. How spread out the change is matters too: 200 lines in one file is easier to review than the same amount across dozens of files.
Ways to split a big change:
- Separate refactoring from behavior changes. Rename and move code in one pull request. Change what it does in the next.
- Stack your changes. Open the first small pull request for review and build the next one on top of it while you wait.
- Split by layer. Database migration first, then the API, then the UI.
- Merge behind a feature flag. Ship unfinished work switched off, so you never need one giant final merge.
Write a description that does the setup work
The reviewer should know what changed and why before reading a single line of code. A template removes the guesswork. On GitHub, saving a file as .github/pull_request_template.md makes every new pull request in the repository start with it:
## What
Adds retry with backoff to the payment webhook handler.
## Why
Provider webhooks fail during their maintenance windows, and we currently drop those events.
## How to test
1. Run the webhook tests.
2. In staging, send a failing webhook and confirm it retries three times.
## Risk
A retry could process the same event twice. The handler skips events
it has already seen (see test "ignores duplicate event").
## Not in this PR
Admin screen for failed events (follow-up PR).Review your own diff first
Before you request anyone’s time, read your own diff in the review tool. Seeing it in that view catches things your editor hides: leftover debug logs, commented-out code, unrelated formatting changes, a missing test. Make sure CI passes. Every problem you catch yourself saves a full round trip.
Handle comments well
- Reply to every comment, even if only with “Done” or a link to the commit that fixes it.
- If a reviewer didn’t understand your code, improve the code or add a comment in it. An explanation that only lives in the review thread won’t help the next person who opens the file.
- Assume good intent. A blunt comment is usually a rushed one, not a personal attack.
- If you disagree, explain your reasoning with specifics. If you’re still stuck after a round or two, talk it through on a call, then write the outcome in the thread so there’s a record.
For reviewers: give useful feedback quickly
Respond fast, without wrecking your focus
Slow reviews stall the author and push them toward bigger, riskier pull requests, because nobody wants to wait twice. Google’s guidance on the speed of code reviews sets one business day as the maximum time to respond to a review request. It also says not to interrupt focused work to do it; wait for a natural break, such as after a meeting or between tasks. If you can’t do a full review soon, say so and give a time.
Read in the right order
- Read the description and any linked issue. Do you agree the change should exist at all?
- Look at the overall design: where the code lives and how the pieces fit together.
- Read the core logic carefully.
- Then go through the rest: tests, configuration, smaller supporting changes.
If you find a major design problem, send that comment right away, before any line-level feedback. Detailed comments on code that’s about to be rewritten waste everyone’s time.
Label how much each comment matters
Authors can’t tell which comments block the merge unless you tell them. Google’s guide suggests prefixes such as Nit: for minor polish, Optional: or Consider: for possible improvements, and FYI: for information that doesn’t need action now. Conventional Comments formalizes the same idea: start each comment with a label such as praise, nitpick, suggestion, issue, or question, and optionally add a decoration such as (blocking) or (non-blocking).
Use your tool’s review states consistently too. On GitHub, a review can be a plain comment, an approval, or a request for changes, as described in GitHub’s docs on pull request reviews. Save “Request changes” for problems that genuinely must be fixed before merging.
The review checklist
Not every row applies to every change, but run through this list until it becomes habit.
| Area | Questions to ask |
|---|---|
| Purpose | Does the change do what the description says? Is it needed at all? |
| Design | Does it belong in this part of the codebase? Does it follow existing patterns, or introduce a new one for a good reason? |
| Correctness | What happens with empty input, null values, very large input, two requests at once, a failed network call, or a retry? |
| Complexity | Could a teammate understand this quickly? Is anything built for a future need that doesn’t exist yet? |
| Tests | Are the important behaviors tested? Would these tests fail if the code broke? |
| Security | Is user input validated? Are permissions checked on the server? Are secrets kept out of code and logs? |
| Performance | Any database queries inside loops, unbounded lists, or heavy work on the UI thread? |
| Naming and comments | Do names say what things are? Do comments explain why, not what? |
| Operability | If this fails in production, will logs or metrics show it? Is there a rollback plan for migrations? |
| Docs | Does a README, API reference, or changelog need updating? |
Leave formatting and style to tools. Once a formatter and linter run in CI, nobody needs to comment on spacing or import order again, and reviewers can spend their attention on the rows above.
Comments that work, and comments that don’t
The same point can land as helpful or hostile depending on how it’s phrased. Good comments are about the code rather than the person, explain why, and make the next step obvious.
| Instead of | Try |
|---|---|
| “This is wrong.” | “issue: If orders is empty, orders[0] throws here. Could we return early for that case and add a test for it?” |
| “Why would you do it like this?” | “question: What led you to poll the API instead of using the webhook? If it’s about missed events, a short comment in the code would help the next reader.” |
| “Use a map.” | “suggestion (non-blocking): A map keyed by user ID would avoid scanning the whole list on every lookup. Probably fine at today’s data size, so your call.” |
| “Fix the formatting.” (repeated on every file) | “Could we add the formatter to CI? Then neither of us has to spend review time on formatting.” |
| “LGTM” on a large diff you skimmed | “I reviewed the API and data model closely and only skimmed the generated migration. Approving on that basis; could someone who knows the schema check the migration?” |
| “We don’t do it that way here.” | “nitpick: The rest of this module uses async/await rather than promise chains. Worth matching for consistency, but not blocking.” |
| Saying nothing about good work | “praise: Pulling the parsing into its own function made these tests much easier to follow.” |
Notice the pattern in the right-hand column: a label that sets the severity, a specific observation, the reason it matters, and a proposed next step that the author is free to improve on.
Team habits that make reviews work
- Write down what reviews are for. Agree on what blocks a merge, what doesn’t, and the expected response time. New team members shouldn’t have to guess.
- Automate what machines check better. Formatting, linting, type checks, and tests belong in CI, not in review comments.
- Route reviews automatically. GitHub can request reviews from the right people when you define code owners in a
CODEOWNERSfile. - Agree on big designs before the code. A short design doc or a quick call before work starts prevents the worst outcome: a finished pull request that needs a different approach. The design phase of the software development lifecycle exists for exactly this.
- Hold AI-generated code to the same standard. The author must understand and be able to explain every line, wherever it came from.
- Let junior engineers review too. Reviewing senior engineers’ code is one of the fastest ways to learn a codebase, and a fresh reader is the best test of whether code is clear.
On distributed teams, reviews are often the main conversation about code, so tone in writing matters even more. The guide to remote work etiquette covers how to keep written feedback clear across time zones.
FAQ
How long should a code review take?
It depends on size and risk. A small, well-described pull request can usually be reviewed properly in one focused sitting. If you can’t get through a change without losing track of what it does, it’s probably too big; ask the author to split it rather than skimming.
How many reviewers does a pull request need?
For most changes, one reviewer who knows the area is enough. Add a second for security-sensitive code, database migrations, payment logic, or changes outside the author’s usual area. On GitHub, repository administrators can require approving reviews before pull requests are merged into protected branches.
Should junior developers review senior developers’ code?
Yes. They learn the codebase and the team’s standards faster, and if a junior can’t follow a change, that’s useful feedback about its clarity. Juniors should feel free to approve, ask questions, and flag anything that looks wrong.
What if the author and reviewer can’t agree?
Move the discussion to a call, since written threads tend to harden positions. Refer to the team’s style guide and agreed standards rather than personal preference. If you still disagree, ask the tech lead to decide, and record the outcome in the thread.
Try this on your next pull request
Add a pull request template to your repository today. On your next review, label every comment as blocking or non-blocking. Those two changes alone remove a lot of everyday review friction. Then share the checklist above with your team and agree on a response time everyone can keep. If you want to see where reviewing fits among the other skills worth building, read about the 10 skills every software engineer needs to grow.