This document compiles modern practices for addressing code review bottlenecks in fast-moving engineering teams, based on practices from companies like Google, Facebook/Meta, Spotify, GitHub, and emerging industry patterns.
- Non-blocking by default: Reviews shouldn't block forward progress
- Time-boxed reviews: Set expectations (e.g., first response within 4 hours)
- Context-rich PRs: Self-documenting changes reduce back-and-forth
- Async-first communication: Written explanations over synchronous meetings
## What
[One-line summary]
## Why
[Business/technical context]
## How
[Implementation approach]
## Testing
[What was tested, how to verify]
## Risks
[What could break, monitoring plan]
## Screenshots/Demo
[For UI changes]- Pre-commit hooks: Catch formatting/linting before review
- CI gates: Don't request review until tests pass
- Self-review first: Author reviews own PR, leaves comments on tricky parts
- Small PRs: Target <400 lines (studies show review quality drops after 400 LOC)
- Reviewable commits: Each commit tells a story, can be reviewed independently
- GitHub: Advanced features (draft PRs, suggested changes, code owners)
- Gerrit: Change-based review (Google's choice)
- Graphite: Stacked PRs made easy
- Reviewable: Enhanced GitHub review experience
- Phabricator: Facebook's tool (now open source)
- GitHub Copilot for PRs: Auto-generates PR descriptions
- CodeRabbit: AI code reviewer (catches bugs, suggests improvements)
- SonarQube: Automated code quality checks
- Codacy: Automated code analysis
- DeepCode/Snyk Code: Security-focused AI review
- Danger: Automates common review comments
- Pronto: Runs multiple code checkers
- ReviewDog: Posts review comments from any linter
- Conventional Comments: Standardizes review comment types
Criteria:
- Docs-only changes (typos, clarifications)
- Test-only additions
- Dependency version bumps (non-major, passing tests)
- Configuration tweaks (with test coverage)
- Automated refactors (IDE-generated)
Requirements:
- All CI checks pass
- Branch up-to-date with main
- No conflicts
- Automated security scans pass
Tools:
- Mergify: Rule-based auto-merge
- Kodiak: GitHub auto-merge bot
- Bors: Testing & merging bot
- GitHub Auto-merge: Built-in feature
Example Mergify Config:
pull_request_rules:
- name: Auto-merge docs changes
conditions:
- author=@team-members
- files~=^docs/
- check-success=CI
- #approved-reviews-by>=1
actions:
merge:
method: squashCriteria:
- Small bug fixes (<50 lines)
- Minor feature additions
- Refactoring within one module
- Low-risk changes to non-critical paths
- Changes by senior engineers
Review Focus:
- Does it solve the stated problem?
- Are there obvious bugs?
- Basic style/readability check
- Trust but verify
Process:
- Use "LGTM with nits" - don't block on minor issues
- Approve + leave comments for follow-up
- Author fixes in follow-up PR
Criteria:
- API/interface changes
- Security-sensitive code
- Database migrations
- Performance-critical paths
- Architecture changes
- Changes by junior engineers (mentoring)
Review Requirements:
- Domain expert approval
- Security review (if applicable)
- Performance testing results
- Documentation updated
- Backwards compatibility verified
Automated Risk Scoring:
risk_score = (
lines_changed * 0.1 +
num_files * 5 +
(50 if touches_auth else 0) +
(50 if touches_db_schema else 0) +
(30 if touches_api else 0) +
(-20 if has_tests else 0) +
(-10 if senior_author else 0)
)
if risk_score < 20:
tier = "auto-merge"
elif risk_score < 50:
tier = "quick-review"
else:
tier = "full-review"A mental model for deciding review approach, popularized by Rouan Wilsenach at Martinfowler.com.
When to use:
- Trivial changes
- High-confidence changes by experienced engineers
- Time-sensitive fixes
- Experimental work that can be reverted
Process:
- Make change
- Merge to main
- Notify team (post-merge notification)
- Monitor for issues
Safety nets:
- Feature flags (can disable quickly)
- Comprehensive test suite
- Good monitoring/alerting
- Easy rollback
When to use:
- Medium-risk changes
- New patterns you want team to learn
- Refactorings
- Default for most work
Process:
- Create PR
- Merge immediately (or within hours)
- Request review asynchronously
- Address feedback in follow-up PRs
Benefits:
- No blocking on reviewers
- Faster iteration
- Review becomes learning, not gating
When to use:
- High-risk changes
- API contracts
- Security-sensitive code
- Areas where you want mentorship
Process:
- Create PR
- Request review
- Wait for approval(s)
- Address feedback
- Merge after approval
Problem: Large features require multiple PRs, but traditional flow blocks each PR on review.
Solution: Stack PRs on top of each other, review in parallel.
PR1 → Wait for review → Merge → PR2 → Wait for review → Merge → PR3
Timeline: 3-5 days
PR1 ← PR2 ← PR3 (all created at once)
↓ ↓ ↓
Review concurrently
Timeline: 1 day
- Graphite: Purpose-built for stacked PRs
- Gerrit: Native support for change chains
- git-branchless: CLI tool for stacked changes
- ghstack: GitHub stacked PRs tool
- Aviator: Stacked PRs + smart merging
- Each PR should be independently valuable: Can merge just PR1, or PR1+PR2
- Keep them small: Stack of 5x100-line PRs > 1x500-line PR
- Clear dependencies: Document which PRs depend on which
- Update downstream PRs: When PR1 changes, rebase PR2, PR3
- Review bottom-up: Review PR1 first, then PR2, etc.
# Create stack
gt branch create "add-user-model"
# Make changes, commit
gt branch create "add-user-api"
# Make changes, commit
gt branch create "add-user-ui"
# Make changes, commit
# Submit entire stack
gt stack submit
# Review happens in parallel
# Merge happens bottom-up automatically- Merge to main/trunk multiple times per day
- Keep feature branches short-lived (<1 day)
- Hide incomplete features behind flags
- Review becomes continuous, not batch
- Release flags: Control feature rollout
- Experiment flags: A/B testing
- Ops flags: Kill switches for performance
- Permission flags: Gradual access rollout
- Development flags: Hide incomplete work
Development → Team Testing → Beta Users → % Rollout → 100% → Remove Flag
- LaunchDarkly: Enterprise feature management
- Split.io: Feature delivery platform
- Unleash: Open-source feature toggle system
- Flagsmith: Open-source with dashboard
- Custom: Simple in-house system (env vars + config)
# Good: Default to safe state
if feature_flags.is_enabled('new_checkout_flow', user_id=user.id):
return new_checkout()
else:
return old_checkout()
# Bad: Flag in the middle of logic
def checkout():
# ... 50 lines ...
if feature_flags.is_enabled('new_step'):
new_logic()
# ... 50 more lines ...- Target: Merge 2-5x per day per engineer
- PR size: <200 lines
- Review time: <1 hour turnaround
# CI pipeline
- Lint & Format Check
- Unit Tests
- Integration Tests
- Build & Package
- Deploy to Preview Environment
- E2E Tests
- Security Scan
- Merge
- Deploy to Staging (with flag off)
- Monitor for issues
- Gradually enable flag- Smaller changes = faster reviews
- Feature flags = can merge incomplete work safely
- Continuous integration = fewer merge conflicts
- Team sees changes immediately
Some work needs to move faster than review can keep up (prototypes, spikes, research).
main (protected, reviewed)
↓
experimental/new-architecture (fast-moving, minimal review)
↓ (periodic sync)
main
- Fast development: Team works on experimental branch
- Daily merges: Pull from main to stay current
- Weekly reviews: Review diffs in batches
- Graduation: When stable, rebase/squash and merge to main
- During experiment: Pair programming, mob sessions (not async review)
- At graduation: Full review of final state + migration plan
if feature_flags.is_enabled('experimental_track'):
return experimental_implementation()
else:
return stable_implementation()- Both implementations in main branch
- Can A/B test in production
- No branch management
- Gradual migration
For radical experiments:
- Fork: Create separate repo/branch
- Experiment: Move fast, break things
- Learnings: Document what worked
- Reimplement: Build production version in main repo with proper review
- Discard experimental code
- No blocking review: Can merge without approval
- Post-merge review: Team reviews 1x per week
- Pair programming: Encouraged for knowledge sharing
- Documentation requirement: Every merge needs explanation
- Rollback plan: Must be trivial to disable
| Tier | First Response | Approval | Merge |
|---|---|---|---|
| Auto-merge | Immediate | N/A | Immediate |
| Quick Review | 2 hours | 4 hours | 6 hours |
| Standard Review | 4 hours | 1 day | 1.5 days |
| Full Review | 8 hours | 2 days | 3 days |
| Urgency | Response | Example |
|---|---|---|
| Critical (P0) | 30 min | Production outage fix |
| High (P1) | 2 hours | Customer-blocking bug |
| Normal (P2) | 4 hours | Feature work |
| Low (P3) | 1 day | Tech debt, refactoring |
- Time to First Review: How fast do reviewers respond?
- Target: <4 hours during business hours
- PR Cycle Time: Time from creation to merge
- Target: <24 hours for 80% of PRs
- PR Size Distribution: Are PRs small?
- Target: 80% under 400 lines
- Review Load Balance: Are reviews distributed evenly?
- Target: No reviewer >30% of all reviews
- Merge Frequency: How often does team merge?
- Target: >2 merges/engineer/day
- Review Speed: Can encourage rubber-stamping
- Number of Comments: Can discourage thorough review
- Approval Rate: Doesn't measure quality
## Weekly Review Health (Team: Payments)
### Cycle Time
- P50: 4.2 hours ✅ (target: <6h)
- P90: 18.5 hours ⚠️ (target: <24h)
- P99: 52.3 hours ❌ (target: <48h)
### PR Size
- <100 lines: 45% ✅
- 100-400 lines: 40% ✅
- 400+ lines: 15% ⚠️
### Review Load
- Alice: 23 reviews (balanced)
- Bob: 8 reviews (low - capacity issue?)
- Carol: 31 reviews (high - bottleneck?)
### Blockers
- 3 PRs waiting >48 hours
- 2 PRs missing required security review# GitHub Actions example
- name: Check Review SLA
if: hours_since_created > 4 && review_count == 0
run: |
# Notify in Slack
# Escalate to team lead
# Add "needs-review" label# Assign reviewers based on:
# 1. Current load (fewer pending reviews = higher priority)
# 2. Expertise (code owners)
# 3. Time zone (available now)
# 4. Learning goals (junior wants to learn this area)
def assign_reviewer(pr):
reviewers = get_eligible_reviewers(pr.files)
reviewers.sort(key=lambda r: (
r.pending_review_count, # Load balancing
-r.expertise_score(pr), # Prefer experts
-r.availability_score(), # Prefer online
))
return reviewers[0]- Review Time: Block 9-10am daily for reviews
- Rotation: On-call reviewer each day (responds to all PRs)
- Pairing: Complex PRs get synchronous review (pair/mob)
- Escalation: PRs blocked >24h auto-escalate to team lead
# .github/workflows/format.yml
name: Auto-format
on: pull_request
jobs:
format:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- run: npm run format
- uses: stefanzweifel/git-auto-commit-action@v4
with:
commit_message: "Auto-format code"Tools:
- Prettier: JavaScript/TypeScript
- Black: Python
- rustfmt: Rust
- gofmt: Go
- clang-format: C/C++
# Pre-commit hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- repo: https://github.com/psf/black
hooks:
- id: black
- repo: https://github.com/PyCQA/flake8
hooks:
- id: flake8Standardize review comments for clarity:
suggestion: Use a more descriptive variable name
This would improve readability.
question: Why did you choose this approach?
I'm curious about the tradeoff here.
nitpick (non-blocking): Extra whitespace
Not important, but noticed it.
issue (blocking): This will cause a memory leak
The listener is never removed.
praise: Nice refactoring!
This is much clearer than before.
Tool: Browser extension or review template
// dangerfile.js
import { danger, warn, fail, message } from 'danger'
// No PR is too small for a description
if (!danger.github.pr.body || danger.github.pr.body.length < 10) {
fail('Please add a description to your PR.')
}
// Big PRs are hard to review
const bigPRThreshold = 400
if (danger.github.pr.additions + danger.github.pr.deletions > bigPRThreshold) {
warn(':exclamation: Big PR. Consider breaking this into smaller PRs.')
}
// Always update tests
const hasAppChanges = danger.git.modified_files.some(f => f.match(/^src\//))
const hasTestChanges = danger.git.modified_files.some(f => f.match(/^tests\//))
if (hasAppChanges && !hasTestChanges) {
warn('This PR modifies app code but no tests were updated.')
}
// Remind about docs
if (hasAppChanges && !danger.git.modified_files.includes('CHANGELOG.md')) {
message('Consider updating the CHANGELOG.')
}GitHub Copilot for PRs
- Auto-generates PR descriptions
- Summarizes changes
- Suggests test cases
CodeRabbit
- Line-by-line AI review
- Catches common bugs
- Suggests improvements
- Learns team's patterns
Sourcery (Python)
- Refactoring suggestions
- Code quality improvements
- Integrated with GitHub
SonarCloud
- Security vulnerabilities
- Code smells
- Technical debt tracking
- Quality gates
# Kodiak config (.kodiak.toml)
[merge]
automerge_label = "automerge"
method = "squash"
delete_branch_on_merge = true
optimistic_updates = true
[merge.message]
title = "pull_request_title"
body = "pull_request_body"
[approve]
auto_approve_usernames = ["dependabot"]- Dependabot: Auto-creates PRs for dependency updates
- Renovate: More configurable than Dependabot
- Snyk: Security-focused updates
Auto-merge safe updates:
{
"packageRules": [
{
"matchUpdateTypes": ["patch"],
"automerge": true
}
]
}- Draft PRs: Signal "not ready for review"
- Code Owners: Auto-assign reviewers
- Required Reviews: Enforce policy
- Suggested Changes: One-click apply
- Review Assignment: Round-robin or load-balanced
New PR from @alice: "Add user authentication" 🔐
Size: 245 lines | Tier: Standard Review
Reviewers needed: @security-team
Review by: 2pm today
[View PR] [Claim Review]
Key Views:
- My Queue: PRs waiting for my review
- Team Queue: PRs needing any review
- Blocked PRs: Waiting >SLA
- My PRs: Status of my submitted PRs
- Trends: Team velocity over time
Tools:
- LinearB: Engineering metrics platform
- Swarmia: Developer productivity
- Haystack: Engineering metrics
- Jellyfish: Engineering management platform
- Custom: Build with GitHub API + Dashboard tool
- Set up auto-formatting: No more style debates
- Install Danger: Automated PR checks
- Define PR tiers: Auto-merge criteria
- Set SLA expectations: Team agreement on response times
- Review time block: Daily 30-min review time
Expected Impact: 20-30% reduction in review time
- Implement Ship/Show/Ask: Train team on model
- Enable auto-merge: Start with docs/tests
- PR size limits: Enforce <400 lines (with exceptions)
- Stacked PRs pilot: Try with one team
- Review rotation: Dedicated reviewer each day
Expected Impact: 40-50% reduction in review cycle time
- AI code review: CodeRabbit or similar
- Feature flags: Infrastructure setup
- Trunk-based development: Move from long-lived branches
- Metrics dashboard: Track review health
- Continuous deployment: Reduce merge-to-production time
Expected Impact: 60-70% reduction in cycle time, 2-3x more deploys/day
- Blameless postmortems: Learn from mistakes
- Review training: Teach effective review techniques
- Pair programming: For complex changes
- Team retrospectives: Continuous improvement
- Celebrate speed: Recognize fast, quality reviews
Expected Impact: Sustained high velocity, improved team satisfaction
- Tool: Gerrit (change-based review)
- Rule: All code reviewed before merge
- Speed: Average 4 hours from upload to submit
- Techniques:
- Owners files (auto-assign reviewers)
- Small changes encouraged
- Tools auto-fix formatting
- Bots provide instant feedback
- Reviewers get metrics on response time
Key Insight: High standards AND high speed are compatible with good tooling.
- Tool: Phabricator (now Sapling/GitHub)
- Speed: Median 2 hours to first response
- Techniques:
- Land/commit directly to main (with review)
- Extensive automated testing
- Easy rollback
- Stacked diffs native to workflow
- Review as learning, not gatekeeping
Key Insight: Trust engineers + great testing = fast iteration.
- Philosophy: Squad autonomy
- Review: Team decides own process
- Common pattern: Ship/Show/Ask with heavy Show usage
- Techniques:
- Feature flags everywhere
- Trunk-based development
- Continuous deployment
- Post-deploy review common
Key Insight: Let teams optimize for their context.
- Speed: Average merge time <1 hour
- Techniques:
- Preview deployments for every PR
- Visual regression testing
- Auto-merge for safe changes
- Small PRs (<100 lines average)
- Review is asynchronous, non-blocking
Key Insight: Make review environment identical to production.
Myth: Slower review = more thorough review Reality: Long review times = context loss, reduced focus Solution: Time-box reviews (30 min max), focus on high-impact issues
Risk: Speed pressure leads to approving without reading Solution:
- Randomized audit of reviews
- Require comments (not just approval)
- Track bug escape rate
- Pair programming for risky changes
Problem: Detailed comments on trivial issues, missing real bugs Solution:
- Educate on what matters (logic > style)
- Auto-fix style issues
- Focus review on: correctness, security, performance, maintainability
Problem: Only 1-2 people can review certain areas Solution:
- Knowledge sharing sessions
- Pair programming
- Documentation
- Gradual ownership expansion
- Accept slightly less expert reviews
Problem: PRs pile up, team gives up on timely review Solution:
- Stop feature work, clear backlog
- Review day (whole team focuses on reviews)
- Tighten SLAs going forward
- Close stale PRs
- Ship / Show / Ask - Rouan Wilsenach (Martin Fowler's blog)
- How to Review Code - Google Engineering Practices
- The Art of Code Review - Palantir Blog
- Trunk Based Development - trunkbaseddevelopment.com
- Software Engineering at Google - Chapters on code review
- Accelerate - Research on high-performing teams
- The DevOps Handbook - Continuous delivery practices
- Graphite: Stacked PRs - graphite.dev
- Gerrit: Google's review tool - gerritcodereview.com
- CodeRabbit: AI code review - coderabbit.ai
- LaunchDarkly: Feature flags - launchdarkly.com
- Danger: PR automation - danger.systems
- Mergify: Auto-merge - mergify.com
- Set up auto-merge for safe changes (docs, tests, deps)
- Auto-format code (eliminate style debates)
- Keep PRs small (<400 lines)
- Set SLA expectations (4-hour first response)
- Use feature flags (merge incomplete work safely)
- Trust over control: Assume good intent
- Speed is quality: Faster feedback = better code
- Continuous improvement: Merge small, iterate fast
- Review is learning: Not just gatekeeping
- Automate everything: Humans for what matters
- Cycle time: PR creation to merge (<24h target)
- PR size: 80% under 400 lines
- Review SLA: 90% get first response <4h
- Deploy frequency: Multiple per day
| Situation | Approach |
|---|---|
| Docs change | Auto-merge |
| Small bug fix | Quick review (Ship/Show) |
| New feature | Show (post-merge review) |
| API change | Ask (blocking review) |
| Security change | Ask (multiple reviewers) |
| Prototype | Experimental branch or feature flag |
| Large feature | Stack PRs (5x100 lines) |
Code review doesn't have to be a bottleneck. By combining:
- Tiered review (not everything needs deep review)
- Automation (catch trivial issues before human review)
- Feature flags (merge safely, deploy gradually)
- Stacked PRs (review in parallel)
- Clear SLAs (set expectations)
- Trust + testing (move fast without breaking things)
...teams can achieve both high velocity AND high quality.
The key is reducing time-to-merge while maintaining or improving quality through automation, testing, and smart risk assessment.
Start with quick wins (auto-format, auto-merge docs), then gradually shift culture toward trunk-based development with continuous deployment.