Git & Collaboration Workflows

Code Review Excellence

18 min Lesson 7 of 28

Code Review Excellence

At companies like Google, Meta, and Stripe, code review is not a formality — it is the primary quality gate before any change reaches production. Engineers at those companies say that a well-run review culture catches more bugs than any automated test suite, spreads architectural knowledge faster than any wiki, and is the main mechanism by which junior engineers learn to think like seniors. This lesson teaches you how to run that process: structuring pull requests for fast, high-quality review; building a feedback culture that does not create defensiveness; and using tooling such as CODEOWNERS and review automation to remove human bottlenecks.

Pull Requests: The Anatomy of a Reviewable Change

The single strongest predictor of review quality is diff size. A pull request with 50–200 lines of changed code gets reviewed carefully; one with 2,000 lines gets rubber-stamped because no reviewer can hold the full context in working memory. At Google, the internal review system (Critique) surfaces a health score that degrades sharply beyond 400 lines. The reason is cognitive: reviewers start skimming, missing logic errors, and approving just to unblock the author.

One concern per PR. Refactoring, feature addition, and bug fix are three separate PRs — not three sections of one monster PR.
Self-review first. Open your own diff in the GitHub UI before requesting reviewers. You will catch 30–40 % of comments before anyone else sees it.
Write a meaningful description. Explain why the change exists, link the ticket, describe the testing approach, and flag any intentional trade-offs. Reviewers should not have to context-switch to Jira to understand what they are reading.
Draft PRs for early feedback. Open a PR in Draft mode as soon as you have a skeleton. This invites architectural discussion before you invest days in the wrong direction.

Pro practice: Large features cannot always be split into a single 150-line PR. Use stacked PRs — each PR targets the previous branch, not main. GitHub and tools like Graphite natively support this. Reviewers read each layer independently; the final merge is a clean squash onto main.

The PR Template: Consistent Context at Scale

A pull request template enforces the minimum context every reviewer needs. Place a PULL_REQUEST_TEMPLATE.md file in the .github/ directory and GitHub will pre-populate the body of every new PR.

## Summary
<!-- One paragraph: what does this change do and why? -->

## Changes
-
-

## Testing
- [ ] Unit tests added / updated
- [ ] Integration tests pass locally (`make test`)
- [ ] Manual smoke-test steps:

## Rollback plan
<!-- How do we revert if this breaks prod? -->

## Related issues
Closes #

Review Culture: Feedback That Improves Code Without Breaking People

Google's internal code review guide distinguishes between nits (minor style preferences that should not block merge), suggestions (optional improvements), blocking comments (must be resolved), and questions (reviewer needs clarification, not necessarily a change). Using these labels in every comment removes ambiguity and prevents authors from spending hours on feedback the reviewer did not even think was mandatory.

Comment on the code, never on the person. "This function has three responsibilities" instead of "You clearly did not think about this."
Explain the why behind every blocking comment. A comment that just says "refactor this" teaches nothing and frustrates the author.
Approve with comments. If the remaining items are nits, approve and let the author decide — do not hold up a deploy over a variable name.
Respond within one business day. Long review queues are the main reason teams abandon the process and merge without review. If you cannot review by end of day, say so and suggest someone else.

Key insight: The goal of a code review is not to find every possible improvement — it is to confirm the change is correct, safe, and consistent with team conventions. Chasing perfection in reviews leads to scope creep, demotivated authors, and PRs that never merge.

CODEOWNERS: Automatic Reviewer Assignment

The CODEOWNERS file (placed in .github/, the repo root, or docs/) maps file path patterns to the GitHub users or teams who own that code. GitHub automatically requests a review from the matching owner whenever a PR touches those paths. This eliminates the manual step of tagging reviewers and ensures that the person with the most context always sees relevant changes.

# .github/CODEOWNERS
# Syntax: <pattern>  <owner> [owner2 ...]
# Last matching rule wins for a given file.

# Default owners for everything not matched below
*                       @myorg/platform-team

# Infrastructure-as-code: infra team required
*.tf                    @myorg/infra-team
.github/workflows/      @myorg/infra-team

# Auth service: security team + lead engineer
services/auth/          @myorg/security-team @alice

# Frontend: two frontend owners
src/frontend/           @bob @carol

# Database migrations: DBA team MUST approve
database/migrations/    @myorg/dba-team

When combined with branch protection rules that require CODEOWNERS approval, you get a hard guarantee: a migration cannot merge without a DBA sign-off, and no one can accidentally bypass it. Configure this in GitHub under Settings → Branches → Branch protection rules → Require review from Code Owners.

CODEOWNERS automatically assigns reviewers; branch protection enforces all approvals before merge.

Review Automation: Let Bots Handle the Mechanical Work

Every comment a human reviewer writes about formatting, import order, or test coverage is cognitive overhead that should be automated. The goal is to make humans focus on logic, architecture, and safety — everything else runs in CI before reviewers even open the diff.

# .github/workflows/pr-checks.yml
name: PR Quality Gates

on: [pull_request]

jobs:
  lint:
    runs-on: ubuntu-24.04
    steps:
      - uses: actions/checkout@v4

      - name: Run ESLint
        run: npx eslint . --max-warnings=0

      - name: Check formatting (Prettier)
        run: npx prettier --check .

  test:
    runs-on: ubuntu-24.04
    steps:
      - uses: actions/checkout@v4

      - name: Run tests with coverage
        run: npm test -- --coverage --coverageThreshold='{"global":{"lines":80}}'

  size-check:
    runs-on: ubuntu-24.04
    steps:
      - uses: actions/checkout@v4

      - name: Warn on large diffs
        uses: codelytv/pr-size-labeler@v1
        with:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          xs_label: 'size/XS'
          s_label: 'size/S'
          m_label: 'size/M'
          l_label: 'size/L'
          xl_label: 'size/XL'
          xl_min_size: 500

Adding a PR size labeler surfaces diff size visually for reviewers. An XL label is a social signal that the PR needs to be split — no policy required, just visibility.

Production pitfall: Do not use "squash and merge" as a universal policy without thought. If a feature PR has 20 commits of meaningful incremental work, squashing destroys the narrative history that future engineers use to understand why a decision was made. Reserve squash for noisy fix-up commits. Google's Critique defaults to keeping meaningful commits and squashing only fixups — replicated by git rebase --autosquash before merge.

Closing the Loop: Review Metrics That Matter

High-performing teams track review health quantitatively. The four metrics to instrument (via GitHub's API or a tool like LinearB or DX) are: Time to First Review (target under 4 hours), Review Turnaround (time from last push to approval, target under 24 hours), PR Cycle Time (open to merge, target under 2 days for most changes), and Review Iteration Count (how many round-trips before approval — more than 3 suggests the description was unclear or the PR was too large). Tracking these makes bottlenecks visible and gives teams a concrete target to improve.

Pro practice: Run a weekly 15-minute async retro on review metrics posted automatically to your team Slack channel. No meeting required — just the numbers, owned by whoever is on call that week. This single habit has reduced PR cycle time by 40–60 % at multiple large-scale teams.