Peer review is the practice of having another engineer inspect a change before it merges into the main branch. It is one of the highest-leverage quality controls in software development because it catches mistakes while they are still cheap to fix.
A good review does more than find bugs. It spreads knowledge, keeps the codebase consistent, and makes sure the change fits the system the team is actually maintaining.
Purpose
Peer review should answer a few concrete questions:
- Does the change solve the stated problem?
- Is the behavior covered by the right tests?
- Does it fit the existing architecture and style?
- Does it change public contracts, data, configuration, or operations?
- Can another engineer maintain this code later?
Review is not a ceremony and it is not a personality test. It is a shared engineering checkpoint before code becomes part of the permanent system.
Pull Requests
The common review mechanism is a pull request. A pull request gives the reviewer a diff, a description, test results, and a place to discuss specific lines.
A good pull request description explains:
- What changed.
- Why the change was made.
- What alternatives were considered, if relevant.
- How the change was tested.
- Any risky areas the reviewer should inspect closely.
The description matters because it becomes part of the project history. Six months later, the pull request often explains the decision better than the final code alone.
Author Responsibilities
The author is responsible for making the review possible.
Before requesting review, the author should:
- Keep the pull request focused on one coherent change.
- Rebase or merge the target branch if the diff is stale.
- Run the relevant tests and checks locally.
- Explain the problem and the chosen solution.
- Call out tradeoffs, skipped work, or known limitations.
- Add tests for new behavior or explain why tests are not appropriate.
- Remove debugging output, temporary comments, and unrelated formatting churn.
A reviewer should not have to reverse-engineer the intent from the diff. Context is part of the author’s job.
Reviewer Responsibilities
The reviewer is responsible for judging the change, not rewriting it from scratch.
A reviewer should check:
- Correctness: does the code do what the pull request claims?
- Behavior: are edge cases, errors, and user-visible outcomes handled?
- Tests: do the tests prove the important behavior?
- Design: does the change fit the existing architecture?
- Maintainability: will the next engineer understand this code?
- Contracts: does it alter APIs, schemas, config, files, or commands?
- Operations: does it affect deployment, performance, logging, or rollback?
The reviewer should separate blocking issues from questions and optional suggestions. A small naming suggestion should not carry the same weight as a data-loss bug.
Giving Useful Feedback
Good review feedback is specific and actionable.
Prefer this:
This handler returns
200even when validation fails. Should this return400so clients can distinguish bad input from success?
Over this:
This seems wrong.
Useful comments usually include the concern, the impact, and the requested change or question. If the issue is blocking, say so clearly. If it is a minor suggestion, label it that way.
Feedback should focus on the code and the system. The goal is to improve the change, not to score points in the review thread.
Responding to Review
The author should respond to every review comment. That response can be a code change, a clarification, or a reasoned disagreement.
Good responses are short and explicit:
Fixed in the latest push.Added a regression test for this path.Good catch, this should return 400.I left this as-is because the caller already normalizes the value here.
If a disagreement reveals a larger design question, move the discussion out of line comments and resolve the decision directly. Review threads are good for focused comments, but they are a poor place for open-ended design debate.
Pull Request Checklist
Before merge, a practical checklist should be boring:
- The pull request has a clear description.
- The change is scoped to one problem.
- Required tests pass.
- New behavior has tests or documented manual verification.
- Public API, schema, configuration, or command changes are called out.
- Documentation or examples are updated when behavior changes.
- The release or rollback impact is understood.
- Blocking review comments are resolved.
- At least one reviewer has approved.
The checklist is not a substitute for judgment. It is a guardrail against forgetting common failure modes.
Common Pitfalls
Review as Gatekeeping
Review should protect the codebase, not establish dominance. If every review feels adversarial, authors will hide risk and reviewers will focus on winning arguments instead of improving the software.
Giant Pull Requests
Large pull requests are slow to review and easy to misunderstand. They also encourage shallow approval because reviewing them properly is exhausting. Slice work into smaller changes whenever possible.
Style Bikeshedding
Style consistency matters, but style debates should be automated or settled in a project convention. Review time is better spent on correctness, behavior, design, and risk.
Vague Comments
Comments like clean this up or this feels weird do not give the author a
clear path forward. Explain the specific concern and what would resolve it.
Approving Without Running or Reading
Approval means the reviewer has done enough work to stand behind the change. For risky code, that may include pulling the branch, running tests, or trying the workflow manually.
Where This Fits
Peer review sits between implementation and release. It depends on good version-control habits and feeds into the release process.
- Version Control Systems explains the branch and history practices that make review possible.
- Test Driven Software Development explains how tests give reviewers something concrete to verify.
- Release Process explains how reviewed and tested changes become a shipped artifact.