top of page

Tutorial 4: Review a Large PR

  • Contributor
  • 5 days ago
  • 2 min read

Ideally, large PRs don't exist. Reality: sometimes they do. Here's how to give them a real review.

Step 1: Ask First (5 min)

Before opening the diff:

  • "Can this be split?"

  • "Is part of this already merged elsewhere?"

  • "Is this one logical change or several stitched together?"

If the answer is "yes, this could be split" — push back. Better than a rushed review.

Step 2: Get the Tour (10 min)

Ask the author for a walkthrough:

- Where's the entry point?
- What's the high-level structure?
- What's intentional vs. mechanical?
- What's risky?
- What part needs the most attention?

15 minutes of orientation saves hours of confusion.

Step 3: Read the Tests First (10 min)

Tests describe intent. Reading them first:

  • Shows what the code is supposed to do

  • Highlights edge cases the author considered

  • Reveals gaps (no test for X)

For a feature PR, tests are the spec.

Step 4: Top-Down Reading (10 min)

Entry point → main flow → branches

Not:

File 1, file 2, file 3, in disk order

Start at the use case. Follow the control flow. Skip implementations until you understand the shape.

Step 5: Multiple Sessions (10 min)

Don't review a 2000-line PR in one sitting:

  • Session 1: high-level review

  • Session 2: deep on tests and core logic

  • Session 3: detail pass on remaining

Spread over 1-2 days. Fresher each time.

Mark progress in comments so you don't reread.

Step 6: Take Notes (10 min)

For yourself:

- Looked at: auth.py, session.py, tests/auth_test.py
- Concerns: race in line 145; missing test for empty input
- TODO: check db migration

Not in PR comments yet. Avoid premature feedback.

After all sessions: distill notes into clean comments.

Step 7: Spot-Check the Mechanical (5 min)

For massive PRs with both logic and mechanical changes (e.g., rename across 200 files):

  • Mechanical: spot-check 5-10 occurrences

  • Logic: deep review

If mechanical looks consistent: trust the diff. The author's automation probably did it right.

Step 8: Run It (10 min)

For a big PR, check it out and run:

git fetch origin
git checkout pr-branch
make test
make run

Run the feature manually. Test edge cases yourself.

Sometimes the bug is visible only at runtime — not in the diff.

Step 9: Bring in Help (10 min)

For 2000+ line PRs touching multiple areas:

  • Pair-review with another engineer

  • Domain expert for sensitive logic (auth, payment, data)

  • Designer for UI

One reviewer is overwhelmed. Many reviewers = thorough review.

Split the file responsibilities; cross-check at the end.

Step 10: Accept Imperfection (5 min)

Massive PRs are hard. You won't catch everything.

Strategies for residual risk:

  • Approve with explicit follow-ups

  • Plan a thorough post-merge review (with the option to amend)

  • Bake monitoring around the launched code

Some risk is inevitable. Mitigate; don't pretend otherwise.

What You Just Did

Large PR review: push back first, orientation, tests first, top-down, multi-session, notes, spot-check mechanical, run it, multi-reviewer, accept residual risk. Hard PRs reviewed responsibly.

Common Failure Modes

LGTM without reading. Bugs ship.

Comment on details; miss the design issue. Diffs are deep but you didn't see the forest.

One reviewer; one sitting. Fatigue → shallow review.

No actual run. Tests pass; runtime fails.

Promise post-merge review; never do it. Backlog grows.

bottom of page