Career
Code Review That Actually Works: A Developer's Process Guide
February 202610 min read
Career
Google's engineering research found that reviewing more than 200-400 lines of code at a time sharply reduces defect detection rates. Yet the average PR in most teams exceeds 400 lines — and the review comment is often 'LGTM' after a cursory scan. Studies show that AI code review tools cut average PR review time from 2-3 hours to 20-30 minutes. These numbers point to the same underlying truth: most teams have a code review process that looks rigorous but isn't. This guide covers how to build a review process that actually finds bugs, transfers knowledge, and ships better code.
The data is clear: defect detection drops precipitously once a PR crosses 400 lines of changed code. Reviewers lose focus, miss context, and start rubber-stamping. Small PRs (under 300 lines) get more thoughtful review, merge faster, and are easier to revert when something goes wrong. The discipline of keeping PRs small forces better code design — features that can't be broken into small PRs usually have design problems that the split exposes. Set a soft limit of 400 changed lines and a hard limit of 800 as a team norm, with exceptions requiring explicit explanation.
Large features don't have to mean large PRs. Decompose work into layers: first PR is the data model and migration; second is the API layer and tests; third is the service logic; fourth is the UI. Each PR is deployable (or at least merge-able behind a feature flag) independently. This approach also parallelizes work — a second developer can start on the service layer while the first developer's data model PR is in review. Feature flags let you merge incomplete features without releasing them to users.
Code Review Process — Healthy Team Flow
Feature Planning
│
┌────▼────────────────────────────────────────────────┐
│ Break feature into reviewable PRs (< 400 LOC) │
│ │
│ PR 1: Data model + migration │
│ PR 2: API routes + validation │
│ PR 3: Business logic + unit tests │
│ PR 4: UI components │
└────┬────────────────────────────────────────────────┘
│
┌────▼────────────────────────────────────────────────┐
│ PR Description: │
│ ✓ What changed (one sentence) │
│ ✓ Why it changed (business reason) │
│ ✓ What I tested │
│ ✓ What is NOT in scope (explicit) │
└────┬────────────────────────────────────────────────┘
│
┌────▼────────────────────────────────────────────────┐
│ Reviewer Checklist (NOT style): │
│ □ Architecture fit │
│ □ Business logic correctness │
│ □ Error handling completeness │
│ □ Security: inputs validated, permissions checked │
│ □ Performance: indexes, N+1 risk │
│ □ Tests cover critical paths │
└────┬────────────────────────────────────────────────┘
│
Target: < 24h review turnaround, < 2 review cyclesFrom my development process for Commsult's ERP modules: write the PR description as if you're explaining the change to a team member who just woke up with no context. Include: what changed, why it changed (the business reason, not just the technical reason), what you tested, and what you're not changing (explicitly) if the scope could be ambiguous. A well-written PR description cuts review time in half — the reviewer spends less time figuring out what they're looking at and more time on the actual review.
Effective code review isn't about style nitpicks — that's what linters are for. Automate style, lint, and formatting checks with pre-commit hooks and CI. Reserve human review capacity for things that automation can't catch: correctness (does this logic actually do what the PR description says?), edge cases (what happens with empty input, null values, concurrent requests?), security (are user inputs sanitized, are permissions checked?), performance (will this query work at scale?), and maintainability (will a new team member understand this in six months?).
## PR Description Template (.github/pull_request_template.md)
### What changed
<!-- One sentence. What does this PR do? -->
### Why it changed
<!-- Business reason, not technical reason.
"Feature X required by finance team" not "added new endpoint" -->
### What I tested
<!-- Describe what you tested manually + what automated tests cover this -->
### Out of scope
<!-- Explicit list of what is NOT in this PR to prevent scope confusion -->
### Documentation
- [ ] I have updated README/API docs for this change
- [ ] New environment variables are in .env.example
- [ ] Database changes are documented in migration file
### Review checklist for reviewer
- [ ] Logic is correct (matches requirements)
- [ ] Error paths are handled
- [ ] No obvious security issues (inputs validated, auth checked)
- [ ] Performance: no N+1 queries, new queries have indexes
- [ ] Tests cover the new behaviorDevelop a mental (or literal) checklist for reviews: Architecture — does this fit the existing patterns? Does it introduce new dependencies that should be discussed? Correctness — does the code match the requirements? Are the business rules correct? Error handling — are all error paths handled? Are errors logged with enough context? Testing — are the critical paths covered by tests? Do tests actually test the behavior, not the implementation? Security — are user inputs validated? Are permissions enforced before business logic? Performance — are database queries indexed? Are there N+1 query risks?
A code review where every comment is a style preference ('I'd use const here', 'rename this variable to X') is not a useful review — it's micromanagement that demoralizes the author and misses real bugs. Prefix style comments with 'nit:' to mark them as non-blocking. Better: agree on a style guide enforced by ESLint and Prettier, and never raise style issues in code review again. Save your review capacity for architectural decisions, logic correctness, and security. The review that says 'this query will cause a full table scan on the users table when the user count exceeds 10K' is worth 100 nit comments.
Fast feedback keeps developer flow high. Engineering guidelines recommend responding to every review within one business day — ideally within a few hours. A PR that sits unreviewed for three days demoralizes the author and creates merge conflicts as other code lands. Build a team norm: if you have a review request, prioritize it over new feature work. A developer waiting for review is blocked; starting new feature work while waiting creates context-switching debt. Some teams use a 'review first' policy — start each day by clearing the PR review queue before picking up new tasks.
Code review is one of the highest-leverage knowledge transfer mechanisms on a team. When a senior developer leaves a thoughtful review explaining why a particular approach is better, that knowledge persists in the PR comments permanently — accessible to any future team member who looks at that code's git history. Cultivate a review culture where the goal isn't just 'approve or reject' but 'make this the best it can be while teaching the author something.' Ask questions ('why did you choose this approach over X?') rather than just demanding changes. Questions surface the author's reasoning and often reveal misunderstandings on both sides.
Measure PR cycle time (time from PR open to merge) as your primary metric — it captures review speed, PR size, and required iteration count together. Target under 24 hours for P2 and below, under 4 hours for urgent fixes. Track review comment density per 100 lines — very low means rubber-stamping; very high means style wars. Track rework rate — percentage of PRs that require more than 2 review cycles. High rework indicates unclear requirements or PR scope problems. Don't track individual review counts or 'reviewer contribution' metrics — they create perverse incentives and toxic review cultures.