Good code review guidelines answer four questions in writing: what counts as ready to approve, what reviewers must check, how feedback is phrased, and how fast a review must happen. This guide gives you a standard you can adopt, a checklist you can paste into a pull request template, and the process steps from submission to merge. It draws on the published practices of Google, Microsoft, GitLab, Metabase, NVIDIA, OWASP and others.
TL;DR
- Approve a change when it definitely improves the overall health of the code, even if it is not perfect.
- Review in a fixed order: description and requirements, design, correctness, tests, security, then readability and style.
- Label every comment as blocking or optional, and explain the reasoning behind each objection.
- Keep pull requests small, acknowledge them within one working day, and automate everything a machine can check.
- Track time to first review and time to merge to improve the process, never to rank individual engineers.
Table of Contents
- Start with the approval standard
- What authors owe the reviewer
- The reviewer checklist
- Security review needs its own pass
- Write feedback people can act on
- The peer code review process, step by step
- Speed: the rule that protects everything else
- Let automation do the mechanical work
- Adapt the guidelines to your team and measure them
- Our perspective
- When your team needs more senior reviewers
- Frequently asked questions
- Sources
Start with the approval standard
Every guideline document needs one sentence that settles arguments. The most widely copied one comes from Google’s engineering practices: reviewers should approve a change once it definitely improves overall code health, even when it is not perfect. The same document says there is no perfect code, only better code, and that reviewers should balance quality against the developer’s forward progress.
This matters because reviews fail in two opposite ways. Some reviewers wave changes through and defects reach production. Others hold every change to a personal ideal, and a two-line fix waits a week. A written standard gives both types the same reference point.
The standard also separates substantive engineering concerns from style preferences. If your team has a style guide, the guide decides. If it does not, a reviewer’s taste is a suggestion, not a reason to block.
What authors owe the reviewer
A review is only as good as the change put in front of it. Authors can do most of the work before anyone else looks.
- Write a description that explains what and why. Google recommends that a change description state what is changing and why, mention known shortcomings, and include background such as bug numbers, benchmark results and design links.
- Keep the change small and focused. Shopify’s engineering team found that small pull requests are easier to start, finish and examine deeply. Its data engineering guideline was roughly 200 to 300 affected lines, published in 2020. Treat that as a starting point and tune it for your codebase.
- Use drafts for early direction. Draft or work-in-progress pull requests let a teammate comment on the algorithm, API or design before you have polished anything.
- Self-review first. Read your own diff as if someone else wrote it. Most leftover debug lines and missing tests show up here.
- Confirm requirements before asking for final approval. GitLab’s documentation says authors should request maintainer approval only after confirming that requirements are satisfied and that no bugs, logic problems or uncovered edge cases remain.
If the requirements themselves were vague, the review will be vague too. Teams that struggle here often fix the problem upstream by learning to write acceptance criteria that can be tested.
The reviewer checklist
A checklist keeps reviewers from spending all their attention on the first thing they notice. Published checklists from different organizations overlap heavily. Atlassian’s guide lists correctness, requirements, tests, security, performance, maintainability, error handling, observability and documentation as the recurring criteria. Sourcegraph’s 2026 checklist recommends checking correctness, readability, tests, architecture and security before going line by line.
Here is a working version, in the order we suggest reviewing.
| Order | Area | Questions to ask |
|---|---|---|
| 1 | Context | Does the description say what and why? Does it match the ticket or requirement? |
| 2 | Scope | Is the change focused on one thing? Are unrelated edits split out? |
| 3 | Design | Does it fit the existing architecture? Is it placed in the right layer or module? |
| 4 | Correctness | Does it do what the description claims? Are edge and failure paths handled? |
| 5 | Tests | Do new tests cover the changed logic? Are they deterministic and fast? |
| 6 | Security | Is input validated? Are authorization checks present? Are secrets managed? |
| 7 | Operations | Are errors logged safely? Is there enough observability to debug it? |
| 8 | Readability | Can a new teammate understand it? Are names and comments clear? |
| 9 | Documentation | Are public interfaces, configuration and runbooks updated? |
Correctness
Sourcegraph’s checklist asks reviewers to confirm the change does what its description says, and to test the awkward inputs: empty, null, zero, negative, very large and concurrent cases. It also asks for real error handling instead of swallowed errors. Microsoft’s engineering playbook adds that reviewers should verify business logic correctness and the correctness of changed tests, not just the production code.
Tests
A test that passes is not automatically a good test. Appfire’s checklist asks that new tests directly cover the changed logic and that they are deterministic, independent and fast. Reviewers who see a retry added to make a test pass should ask why it fails in the first place. Our write-up on how to fix flaky tests before blind retries hide the root cause covers that case.
Reading the whole change
Microsoft’s guidance is blunt about thoroughness: reviewers should read every changed line, look at the surrounding code when the diff is not enough, and ask the author to clarify anything they do not understand. Approving code you did not understand is the most common way a review becomes a formality.
Security review needs its own pass
A general checklist catches obvious issues. For code that handles authentication, payments, personal data or external input, add a security pass. The OWASP Secure Code Review cheat sheet recommends looking at entry points and input validation, authentication and authorization, data flow and business logic, cryptography, error handling and logging, configuration, dependencies and trust boundaries.
OWASP also makes a point worth putting in your guidelines: manual review complements automated static and dynamic scanning, because people are needed for business logic, complex security controls and context-specific attack paths. A scanner cannot tell you that a discount endpoint lets any logged-in user apply another customer’s coupon.
A practical rule is to name a security reviewer for any change touching trust boundaries, and to say so in the pull request template.
Write feedback people can act on
Most review conflict comes from wording, not from technical disagreement. The guidelines should set a few rules for comments.
- Explain the objection. NVIDIA’s guidelines tell reviewers to explain their reasoning and to label what is a blocker and what is optional. A comment that says only to change something teaches nothing.
- Separate blocking from non-blocking. Prefix comments with a convention such as blocking, suggestion or nit. The Software Engineering Guide recommends separating blocking issues from suggestions for exactly this reason. For a deeper look at prioritizing significant problems over optional suggestions, see this analysis of review priorities.
- Prioritize by impact. NVIDIA’s checklist puts user-facing behavior first, technical correctness second and code quality third. When only non-blocking quality concerns remain, it recommends approving and handling them separately.
- Comment on the code, not the person. Write about what the code does, not what the author failed to do.
- Move to a conversation when a thread stalls. If a topic is ambiguous or contentious, a ten-minute call beats fifteen comment rounds. Record the outcome in the thread.
- Use hard vetoes sparingly. Metabase treats an explicit -1 as a veto that must be resolved before merge, and reserves it for things like missing tests, major style violations or broken assumptions.
The Academy Software Foundation frames the principle well: reviews should explain objections, invite counterarguments, and never close a pull request without dialogue, except for basic nonconformance or abandonment. Review is a dialogue aimed at improving the change, not a gate guarded by one person.
Handling disagreement
When author and reviewer disagree, the guidelines should say who decides. A workable order: first, apply the approval standard and the style guide. Second, if it is still a question of design, ask a third engineer or the code owner. Third, record the decision so it does not return next week. Do not let the louder or more senior voice settle it by default.
The peer code review process, step by step
The steps below match the workflow most teams use on GitHub, GitLab or similar tools.
- Author opens the pull request with a description, linked ticket and test evidence. Draft status is fine for early feedback.
- Automated checks run. Formatting, linting, tests and security scans must pass before human review begins.
- Author requests review from the right people. Metabase requires at least one team-engineer approval for any significantly complex change, and review by engineers whose work is affected.
- Reviewer acknowledges within one working day, even if the full review comes later.
- Reviewer works through the checklist, leaving labeled comments.
- Author responds, either changing the code or explaining why not. The Academy Software Foundation describes the ideal flow as a non-author who critiques and approves the change, with the author making the requested changes, or approving changes the reviewer made, before merge.
- Reviewer re-checks only what changed and approves when the standard is met.
- Author merges and tracks any follow-up work as separate tickets.
Keep adjacent problems out of the current change
While reading, reviewers often spot an old bug or a piece of technical debt near the diff. Microsoft’s guidance is to keep attention on the change’s scope and track those findings as separate work instead of blocking the current pull request. Create the ticket, link it in a comment and move on.
Speed: the rule that protects everything else
Slow reviews cause large pull requests, because authors batch work to avoid waiting repeatedly. Large pull requests then get shallow reviews. The loop feeds itself.
The Software Engineering Guide suggests monitoring time to first review and time to merge, with a target such as responding within one working day. Put that number in your guidelines and treat it as a team commitment. A reviewer who cannot do a full review today can still say so and give a date.
The approval standard supports speed. If a change improves the code and has no blocking issues, merge it. Polish can come later in a follow-up.
Let automation do the mechanical work
Every minute a human spends on indentation is a minute not spent on design. Microsoft’s playbook says linters can automate parts of review so that people can focus on architectural and functional correctness. The Software Engineering Guide extends the idea: automate formatting, linting, tests and security scans, and reserve human attention for design, correctness, meaningful tests and system fit.
If you run GitLab with static analysis, this overview of GitLab and SonarQube integrations shows how those checks can feed into the delivery workflow.
AI-assisted review
Automated and AI review tools are now common. The Academy Software Foundation’s position is that they may augment but not replace human review, and that the human author remains responsible for any suggestion they incorporate. Write that into your guidelines: a bot comment is input, and a named person owns the merge.
Adapt the guidelines to your team and measure them
A five-person startup and a regulated enterprise should not use identical rules. GitHub’s guidance on scaled governance suggests setting rules at enterprise, organization and repository levels, so universal controls sit at the top and legitimate local needs are handled lower down. It also recommends evaluating new rules in non-blocking mode first and tracking coverage, enforcement, friction, time to merge, bypasses and violations.
For a small team, a simple version works:
- Risk tiers. Payment code and authentication need two approvers and a security pass. Documentation changes need one.
- Pilot new rules. Run a rule as advisory for two weeks before it blocks merges.
- Metrics. Watch time to first review and time to merge. The Software Engineering Guide’s view is that latency and merge data should be used to improve the process rather than judge individuals.
- Retrospective. Every quarter, read five recently merged pull requests and five that stalled. Ask what the guidelines failed to say.
Review also does not replace testing before release. A reviewer reads code; someone still has to use the product. Our software release checklist shows where human checks sit alongside code review, and the comparison of UAT and QA explains who signs off on what.
Our perspective
We place senior engineers inside growth-stage teams, so we see the same pattern repeatedly. A new engineer joins, the code is good, and the friction comes from unwritten review habits: how large a pull request should be, who approves, what a nit is, how quickly someone must answer. Written guidelines remove most of that friction in the first month.
We also see founders treat review as a quality filter only. It is equally the main way a new engineer learns the codebase and the team’s judgment. That is why we favor a short, specific document over a long one, and why we ask distributed teams to agree on response times across time zones, not just on rules for the code.
Our advice for a team adding remote or offshore engineers: give them the same review standard as everyone else, explain the reasoning behind it, and measure turnaround rather than hours online. Trust follows from clear expectations and visible results, a point we expand in our guide to managing offshore developers without micromanaging.
— Editorial Team
When your team needs more senior reviewers
Guidelines only work when there are enough experienced people to apply them. If review has become the bottleneck on your team, we can help. Our staff augmentation service places senior, pre-vetted Eastern European engineers in your team from $45 per hour, and they review and write code under your standards. If you want to confirm a build before shipping, our Human Release Check is $1,500 one-off.
Frequently asked questions
What should a code review checklist include?
Cover context and requirements, scope, design, correctness including edge cases, tests, security, observability, readability and documentation. Order matters: check the big questions about design and correctness before style. The table above gives a version you can paste into a pull request template.
How long should a code review take?
Acknowledge the request within one working day at most, and finish the first full review soon after. Smaller changes are reviewed faster and more thoroughly, and Shopify’s 2020 guideline pointed to roughly 200 to 300 affected lines. Measure time to first review so you can see where changes wait.
Who should approve a pull request?
At least one engineer other than the author, and anyone whose work the change affects. For high-risk areas such as authentication or payments, require two approvers and a named security reviewer. Define these tiers in writing so they do not depend on who is online.
Is code review obsolete now that AI tools exist?
No. AI and automated tools can handle mechanical checks and surface suggestions, but the Academy Software Foundation says they augment human review rather than replace it. People still judge design, business logic and fit with the system, and the author stays responsible for what merges.
Sources
- The Standard of Code Review, Google eng-practices
- Writing good CL descriptions, Google eng-practices
- Reviewer Guidance, Microsoft Engineering Fundamentals Playbook
- Great Code Reviews, Shopify Engineering
- Code Review Guidelines, GitLab Docs
- What Are Code Reviews, Atlassian
- The Code Review Checklist for Engineering Teams, Sourcegraph
- The ultimate code review checklist, Appfire
- Secure Code Review Cheat Sheet, OWASP
- Code Review Guidelines, NVIDIA
- Code Review Best Practices, Academy Software Foundation TAC
- Code reviews, Metabase Documentation
- Code review and collaboration, Software Engineering Guide
- Tuning scaled governance, GitHub Learn
Recommended
- Legacy Code Onboarding: A Practical Plan for New Engineers
- How to Write Acceptance Criteria That Teams Can Test
- Technical Screening for Developers: A Practical Guide
- Software Release Checklist: 5 Gates, Rollback, and Human Checks
This article may have used AI tools in the writing process. Facts, figures, and recommendations can be wrong or out of date. Do not treat this as professional advice.
