Command Palette

Search for a command to run...

Hectal
PHASE 4Intermediate ~14 min· topic 2 of 4

Topic 4.2

Code Review Workflow & Pull Request Etiquette

In one line

A pull request is a conversation, not just a diff — small, focused PRs with a clear description get reviewed faster and better than large, unexplained ones, on both sides of the review.

0/4 · 0%

Think of it like this

Handing someone a single, clearly-labeled box to inspect versus dumping an entire truckload of unsorted items in front of them and saying 'check this.' A small, focused pull request is the labeled box — genuinely reviewable in a reasonable amount of time; a massive, sprawling one all but guarantees a shallow, rushed review.

Key ideas

  1. 01

    A good pull request DESCRIPTION explains the WHY, not just repeats the diff — what problem this solves, why this particular approach was chosen (especially if a less obvious one was rejected), and anything a reviewer should specifically pay attention to (a tricky edge case, a deliberate trade-off). The code itself already shows WHAT changed; the description's job is everything the diff can't say on its own.

  2. 02

    SMALL, FOCUSED pull requests (ideally reviewable in under 15-20 minutes) get better reviews than large ones, for a genuinely simple reason: reviewer attention and thoroughness measurably degrade as diff size grows. A large feature is often better split into several smaller, sequentially-mergeable pull requests than submitted as one enormous one.

  3. 03

    As a REVIEWER, the goal is catching genuine problems (bugs, unclear logic, missed edge cases, security concerns) — not enforcing personal style preferences the team hasn't actually agreed on. Distinguishing 'this is objectively a bug' from 'I would have personally written this differently' is a genuinely important review skill, and most teams reserve the former as blocking and the latter as optional/non-blocking ("nit:" comments).

  4. 04

    As an AUTHOR, responding to review feedback defensively (or silently ignoring it) is far less productive than treating it as genuinely useful — even feedback that turns out to be wrong is worth a clear, respectful explanation of why, both for that reviewer and for anyone reading the conversation later. Review threads are also documentation of WHY a piece of code ended up the way it did.

  5. 05

    Most real teams gate merging on both a human APPROVAL and passing AUTOMATED CHECKS (CI — tests, linting, build success, Phase 7) — the automation catches objective, mechanical problems fast and consistently, freeing human reviewers to focus their limited time and attention on the things only a human can actually judge: is this the right approach, is this genuinely correct, is this readable.

Code & diagrams

good-pr-description.mdmarkdown

The shape of a description that actually helps a reviewer, versus one that doesn't.

## Poor description
"Fixed the bug."

## Genuinely useful description

### What
Fixes a race condition where two concurrent requests could both pass the
"email not taken" check before either had committed, resulting in duplicate
accounts with the same email.

### Why this approach
Considered a unique DB constraint alone, but that surfaces a raw SQL
exception to the user on conflict — added an application-level check-then-
insert inside a transaction with SELECT ... FOR UPDATE, so the failure path
returns a clean, expected "email already taken" error instead.

### What to look at closely
The transaction isolation level change in `db/config.py` — bumped from
READ COMMITTED to REPEATABLE READ for this specific code path only, not
globally, since a global change would affect unrelated queries' performance.

### Testing
Added a test that fires 50 concurrent signup requests with the same email
and asserts exactly one account is created.

Explain it without notes

01

Why does a pull request's description matter even though the reviewer can already see the full diff?

02

As a reviewer, how do you decide whether a piece of feedback should block the merge versus being an optional suggestion?

Practice

01

Take any recent piece of your own work (or a hypothetical one) and write a pull request description following the What / Why / What to look at / Testing structure from this topic's example.

02

Find a real, well-reviewed pull request on a popular open-source project on GitHub and read through its review comments — identify at least one comment that was clearly blocking versus one that was clearly an optional style suggestion.

Trade-offs

  • ↔

    Splitting large work into several small, sequential pull requests produces measurably better reviews and faster turnaround, but it does require more upfront planning about how to slice the work into independently reviewable (and often independently mergeable) pieces — for genuinely small, self-contained changes, this overhead isn't worth it, but for anything touching multiple files or concepts, it usually pays for itself in review quality.

Done when you can

  • I can write a pull request description that explains why, not just what.

  • I understand why small, focused pull requests get better reviews than large ones.

  • I can distinguish blocking review feedback from optional style suggestions, as both a reviewer and an author.