✍️ English & Grammar · Chapter 7

Writing a clear PR description

The diff says what changed. Only you can say why — and reviewers approve what they understand.

Study this lesson free in the app →

The basics

A PR description is the short note attached to a pull request that tells a reviewer what the change does, why it exists, how it was checked, and what could go wrong.

The problem it solves: the diff shows what changed, never why. Without a description, every reviewer spends twenty minutes reconstructing context you already have — so the PR sits for three days, and the review that finally happens is spread thinly over 14 files instead of focused on the risky part.

The mental model: handing someone a marked map instead of a satellite photo. You move the reconstruction work from every reviewer to the one person who already knows the route: you.

Four questions, four short paragraphs:

1. What does this change? One plain sentence.
2. Why now? The bug, ticket or number that forced it.
3. How to verify — the command you ran, the data, the before/after numbers.
4. Risk — what breaks if it's wrong, how to roll back, and what you're unsure about.

Explained

Title: fixes

Description: (empty)

Files changed: 14

This PR will sit for three days. Not because it's wrong — because opening it costs the reviewer twenty minutes of reconstruction before they can even start reviewing.

A good description doesn't add work. It moves work from every reviewer to the one person who already has the context: you.

Handing someone a map

You've walked a route. Now you ask a friend to check it's safe.

You can hand them a satellite photo and say "have a look". They'll spend most of their time working out where you went and why, and very little on whether it was safe.

Or you hand them a marked map: here's where I started, here's why I avoided the river, here's the bit I'm unsure about.

Same walk. In the second case they spend all their attention on the actual question — and they look hardest at the bit you flagged, which is usually where the problem is.

The four questions every description answers

Four questions, four short paragraphs. Anything longer than a screen usually means the PR itself should be split.

The title carries most of the weight

Titles appear in the PR list, the merge log, release notes and git log forever. They're read far more often than the description.

Format: imperative verb, object, under ~70 characters. Say what it does, not what you did.

✗ fixes
✗ Updated pipeline.py and added some tests
✗ Bug fix as discussed in standup
✓ Dedup ride events on event_id before aggregation
✓ Add DLQ for unparseable payloads with reason codes
✓ Partition raw landing by event date, not ingest date

The imperative reads naturally as "this commit will [title]" — which is exactly what a merge does.

A useful test: if someone reads only your title in six months while bisecting a bug, does it tell them whether to look here? "fixes" fails. "Dedup ride events on event_id" passes.

The shape, filled in

## Dedup ride events on event_id before aggregation

**What**
Adds a deduplication step keyed on `event_id` between validation and
the daily revenue aggregation.

**Why**
Kafka gives us at-least-once delivery, so a consumer retry re-delivers
the same event. Friday's revenue was overstated by ~2.4% (₹1.1L) because
retried `ride_completed` events were counted twice. Ticket: DATA-412.

**How to verify**
- `pytest tests/test_dedup.py` — covers duplicates, empty batch, nulls
- Ran against Friday's raw partition: 4.02M events in, 3.93M out,
  and the total now reconciles with the payment provider to 0.02%.

**Risk / notes**
- Dedup is in-memory per batch, so duplicates split across two batches
  still slip through. Acceptable for now (retries land within seconds);
  the durable fix is a unique index on the sink, tracked in DATA-418.
- Rollback is safe — remove the step, no schema change.
- **Please look closely at** the tie-break when two events share an
  `event_id` but differ in `fare`. I keep the later one; I'd like a
  second opinion on whether that's right.

The last bullet is the highest-value line in the whole thing. Telling a reviewer where you're unsure directs their attention to where bugs actually are — and it's the opposite of the instinct to look confident.

The same PR, two ways

Version A

fixed the double counting issue

The reviewer opens 14 changed files. They don't know what double counting means here, whether it was reported or noticed, how big it was, or what "fixed" was verified against. They ask three questions in comments. You reply over two days across timezones. It merges on Thursday.

Version B — the description above.

The reviewer reads for ninety seconds, sees the numbers reconcile, goes straight to the tie-break logic you flagged, and finds a genuine edge case: two events with the same event_id and different fares shouldn't silently pick one — that's a producer bug worth alerting on. Merged Tuesday with a better outcome.

The description didn't just save time. It changed what was found, because it pointed the reviewer's attention at the part that deserved it rather than spreading it evenly over 14 files.

What's missing?

## Fix Airflow DAG

Changed the schedule and updated the retry logic as discussed.
Also did some cleanup.

Tested locally, works fine.

Size is the other half of reviewability

No description rescues a 2,000-line PR. Review quality falls off a cliff with size — past a few hundred lines, reviewers stop finding logic errors and start commenting on naming, because that's all the attention they have left.

What actually makes PRs reviewable:

• One concern per PR. If the title needs an "and", split it
• Separate mechanical from meaningful. A rename touching 40 files and a logic change should never share a diff — put the rename in its own PR and say so
• Stack them. A big change becomes 4 small PRs in sequence, each reviewable in ten minutes
• Say what you deliberately didn't do. "Not adding the unique index here — tracked in DATA-418" prevents the review comment you'd otherwise get, three times

And a rule that has nothing to do with writing: review your own diff before opening the PR. You'll find the debug print, the commented-out block and the missing test — and finding them yourself costs one minute rather than one round trip.

Words to know

In two minutes

• The diff says what changed; only the description says why — and reviewers approve what they understand.
• Answer four questions: what, why, how to verify, what's the risk.
• Titles are read in git log for years: imperative verb, specific object, under ~70 characters.
• Verification means a command, data and numbers — never 'tested locally, works fine'.
• Flag the part you are unsure about; it focuses the reviewer where bugs actually are.
• Name blast radius and rollback for schema, schedule and config changes.
• One concern per PR: if the title needs 'and', split it; keep mechanical renames separate.
• Self-review your diff before opening the PR — it costs one minute instead of one round trip.

Test yourself

There are 11 questions, 3 puzzles and 7 recall cards for this lesson in the app — the recall cards come back on later days, just before you would forget.