Pull Requests
By the end of this lesson
Propose changes in a reviewable, well-described form.
A pull request is a proposal: take the commits on this branch and add them to that branch. It is not a Git feature. Git has no idea what a pull request is — GitHub, GitLab and similar services built the concept on top of branches, and GitLab calls it a merge request.
What the service adds is a place for the change to be discussed while it is still changeable: the list of commits, the combined change in reviewable form, automated check results, and a conversation attached to specific lines.
The lifecycle of a typical change, from branch to merged:
Branch from an up-to-date main
Fetch first, then create your branch. Starting from a stale main is the most common cause of avoidable conflicts later.
Commit in focused pieces
The commits become the reviewer's reading order. A sequence of small, well-described commits is far easier to review than one large one, even when the total change is identical.
Push the branch and open the pull request
Choose the branch it should merge into, then write the title and description. If the work is incomplete and you want early feedback, open it as a draft so nobody reviews it as finished.
Let the automated checks run
Build and tests run against the proposed result. Fix failures before asking for review — a reviewer should not be the one to discover that the tests fail.
Respond to review with new commits
Push follow-up commits rather than rewriting history while people are reading. The reviewer can then see what changed since their comment.
Merge, then delete the branch
Once it is approved and checks pass, merge it. Deleting the branch afterwards removes a pointer, not the work.
# Start from the current state of the shared branch
git fetch origin
git switch -c reject-negative-quantities origin/main
# ... work, committing in focused pieces ...
git commit -am "Reject order lines with a quantity below one"
git commit -am "Add tests covering zero and negative quantities"
# Read your own change before anyone else has to
git diff origin/main...HEAD
# Push and set the upstream so later pushes need no arguments
git push -u origin reject-negative-quantities- Creating the branch from origin/main rather than from your local main guarantees you start from what the remote actually has.
- The three dots in origin/main...HEAD show your branch's changes measured from the point where it diverged. This is close to what the reviewer will see, and reading it yourself catches stray edits and leftover debugging.
- git push -u records the upstream, so subsequent pushes are a bare git push.
- After this, the pull request is opened in the web interface, or with a command-line tool if your team uses one. The Git side of the work is finished.
The description is where you save the reviewer's time
A reviewer arrives knowing nothing about your last two days. A useful description answers four things:
- What this changes, in one or two sentences a non-author can follow
- Why it is being changed — the bug, the request, the problem you hit
- How you approached it, and anything you considered and rejected
- How you verified it, and anything you deliberately left out of scope
Reject order lines with a quantity below one
Orders submitted through the bulk import could contain a quantity of
zero or a negative number. Those lines reached the invoice calculator
and produced totals that were correct arithmetically and wrong in every
other sense. Two credit notes last month traced back to this.
Validation now happens in OrderLineValidator, at the point the import
is parsed, so both the API and the import path are covered by one rule.
I considered rejecting the whole import when any line is invalid, but
chose to reject individual lines and report them, because partial
imports are how the operations team currently works.
Verified: new unit tests for zero, negative and boundary values, plus a
manual import of the sample file attached to the original report.
Out of scope: the duplicate-line problem in the same report, which
needs a separate decision about matching rules.- The first paragraph is the reason, including what it cost. That is what tells a reviewer how carefully to read.
- The third paragraph is the part most descriptions omit and reviewers most often need: an alternative you considered and why you did not take it. It prevents a round of review spent asking about a decision you already made.
- Naming what is out of scope stops the review expanding into work you deliberately deferred.
Summary
- A pull request is a hosting-service feature built on branches — Git itself has no such concept
- Branch from an up-to-date main, commit in focused pieces, and read your own change before requesting review
- The description should cover what, why, the approach including alternatives rejected, and how it was verified
- Small proposals get read; large ones get skimmed, so split refactors from behaviour changes
- Add commits during review rather than rewriting history under the reviewer
Practice
Attempt each one before opening the solution. Getting it wrong first is how the idea sticks.
Try it yourself
Write the description first
Take a change you have made recently, or one you are about to make, and write the description before writing any code: what, why, how, and how you will verify it.
Then make the change and compare. Did the description survive contact with the work?
Show solution
Writing it first tends to expose an unclear scope while changing the scope is still cheap. If you cannot state why in two sentences, the work needs a conversation more than it needs code.
It also produces a better description than writing one afterwards, when you have stopped noticing which parts were non-obvious.
Think about it
Think about it
A pull request changes 900 lines across 30 files: a dependency upgrade, a rename affecting most of them, and a two-line bug fix.
It gets an approval within four minutes. What went wrong, and how should it have been split?
Show solution
Four minutes is not enough to review 900 lines, so the approval means the reviewer skimmed it. Nobody has meaningfully checked the two lines that actually change behaviour.
Three proposals would work: the dependency upgrade on its own, so any fallout from it is isolated; the rename on its own, verifiable as a no-op; and the bug fix, small enough to review properly and to revert cleanly if it is wrong.
Note that the total work is identical. Splitting it costs a little branch management and buys an actual review.
Saved in this browser only.