Steven's Knowledge

Opening a Pull Request

How to author a pull request that is easy to review — scoping, description, commit hygiene, self-review, and responding to feedback

Opening a Pull Request

Half the quality of a review is decided before the review starts. A reviewer has minutes against the hours the author spent; everything the author does to close that gap — a tight scope, a clear description, a clean history — buys back review attention for the things that actually need judgment.

A pull request is not a delivery mechanism for code. It is a request for someone else's time and a permanent record of why the change exists. Author it for the reader.

The companion to this page is Code Review, written from the reviewer's side. The two describe the same loop from opposite ends.

What the Author Owes the Reviewer

A reviewable PR gives the reviewer four things before they read a single line of the diff:

  1. A reason. What problem does this solve, and why now? A link to the issue is a start, not a substitute.
  2. A boundary. One change, scoped so the reviewer can hold all of it in their head at once.
  3. A map. Where to start, what to focus on, what the author is unsure about.
  4. A clean history. Commits that tell the story of the change, not the story of the afternoon.

Skip any of these and the reviewer reconstructs it themselves — slowly, and from guesses.

Keep It Small

The single highest-leverage thing an author controls is size. Review quality falls off a cliff past a few hundred lines: a 50-line PR gets line-by-line scrutiny, a 1,000-line PR gets "LGTM." Reviewers are human, and a wall of diff trains them to skim.

  • One PR, one thing. A bug fix is a PR. A refactor is a PR. A bug fix and the refactor that enabled it are two PRs — the refactor first, behavior-preserving, then the fix on top.
  • Separate mechanical from meaningful. A rename touching 80 files plus three lines of real logic hides the logic. Land the rename alone, then the logic in a diff a reviewer can actually read.
  • Stack when the work is genuinely sequential. A large feature becomes a series of small PRs, each building on the last, each reviewable on its own. The reviewer approves a chain of comprehensible steps instead of one incomprehensible leap.

A PR over ~800 lines is itself a smell. Before opening it, ask whether it is really one change or several wearing a trench coat.

Write the Description

The description is the part of the PR that outlives the review thread. A future engineer running git blame finds this text, not the back-and-forth in the comments. Write it for them.

A template that scales from a one-line fix to a feature:

## Why
The problem, in terms a reviewer who lacks your context can follow.
Link the issue, but state the gist here too.

## What
The approach. Not a restatement of the diff — the shape of the
solution and the key decisions inside it.

## How it was tested
What you ran, what you saw. "Added unit tests" / "manually verified
on staging with X" / "ran the migration against a prod snapshot."

## Notes for the reviewer
Where to start. What you're unsure about. Anything you considered
and rejected, so they don't re-suggest it.

The last section is the one most authors skip and reviewers value most. "I wasn't sure whether to handle the retry here or one layer up — thoughts?" turns a reviewer from a fault-finder into a collaborator, and points their attention exactly where it pays off.

Keep the History Clean

The diff is what changed; the commits are why, in order. A reviewer who can read the change as a sequence of intentional steps reviews faster and trusts more.

  • Atomic commits. Each commit does one thing and leaves the tree working. "Fix typo" squashed into "Add caching layer" robs both of meaning.
  • Messages that explain why. The subject says what; the body says why this way and not another. "Use a WeakMap to avoid leaking listeners on unmount" beats "fix memory bug."
  • Squash the noise. "wip," "fix lint," "address review" are afternoon noise, not history. Clean them up before requesting review, or rely on a squash-merge to collapse them — but never make the reviewer wade through them.

History is a courtesy to the reviewer today and to the archaeologist next year. They are usually the same person.

Review It Yourself First

Open your own diff in the review tool and read it as a stranger would. You will catch the leftover console.log, the commented-out block, the file you didn't mean to commit, the function you renamed in one place but not another. Every one of these you find is one the reviewer doesn't waste a comment on.

This is also the right place to run an AI review — privately, before anyone else sees it. The AI is a thorough, tireless first pass at exactly the boring mistakes you skim past in your own code: the unhandled null, the missing await, the boundary you forgot. Fix what it finds, ignore the noise, and the diff a human finally sees is already a level cleaner.

The self-review is not optional polish. A PR you have not read yourself is a PR you are asking someone else to read for you.

Open a Draft for Direction

When the change is architecturally significant or you are unsure of the approach, open a draft PR early — before it is finished. A reviewer can redirect 200 lines cheaply and 2,000 lines expensively. Getting "this whole approach should be different" after two days is far better than after two weeks.

Mark it clearly as a draft, say what kind of feedback you want ("direction only, not line-level yet"), and convert it to a real PR once the shape is settled. Asking for design feedback on a polished, complete PR invites the worst outcome: the reviewer sees the effort already spent and waves through a design they would have pushed back on earlier.

Respond to the Review

The reviewer points at problems; the author has to resolve them without taking the criticism personally. The discipline that works:

  • Assume good intent. A blunt comment is a comment, not an attack. Tone is hard to read on a screen; read it generously.
  • Respond to every thread. Even "good point, fixed." Silence reads as ignoring, and forces the reviewer to check whether you saw it.
  • Push back when you disagree. A reviewer can be wrong, or lack context you have. A change applied without conviction becomes a maintenance burden owned by no one. Disagree with a reason, not with silence.
  • Split the PR when the review reveals two issues. If a reviewer says "this should be two PRs," they are usually right. The cost of splitting now is smaller than the cost of a muddled change in the history forever.
  • Update the description as the change evolves. The future reader does not have the review thread; they have the description. If the approach changed mid-review, the description has to change with it.

The author's discipline closes the loop the reviewer opened: the reviewer raised the bar, the author met it without resentment. That is how the standard holds on both sides.

Before You Request Review

A quick pass before clicking the button:

  • The PR is one change, scoped small enough to review in one sitting.
  • The description states why, what, and how it was tested — in plain terms.
  • Notes for the reviewer point at where to start and what you're unsure about.
  • Commits are atomic and their messages explain why, not just what.
  • You read your own diff and ran it; no stray logs, dead code, or unintended files.
  • CI is green before a human is asked to look, not after.
  • Tests cover the new behavior, or the description says why not.

If you would not want to review this PR yourself, do not ask someone else to.

On this page