Pull Request Best Practices: Size, Scope and Review Velocity

Lesson 12 of 12Intermediate → Advanced14 min readGitHub Engineering · Pull RequestsVerified: gh 2.98.0, August 2026

Most pull request problems are one problem wearing different hats: the change was too big.

Slow review, superficial review, merge conflicts, stale approvals, risky deploys and reverts that take half a day all become dramatically less likely as pull requests get smaller. Almost everything else in this lesson is downstream of that.

This is a decision guide rather than a rulebook. Where advice is commonly stated as a number, this lesson gives reasoning instead — because the numbers people quote are usually invented, and the reasoning transfers to your situation in a way an invented number does not.

Size: the mechanism, not a number

You will see confident claims that pull requests should be under 200, 250 or 400 lines. Those figures circulate widely and are rarely traceable to anything. Rather than repeat one, here is why size hurts, which lets you judge your own case.

Attention is finite and does not scale linearly. A reviewer holds a limited amount of context. Up to some threshold they can reason about the whole change. Past it, they review the diff locally — line by line, checking each hunk in isolation — and stop reasoning about whether the change is correct as a whole. That transition is where review stops catching design problems and starts catching typos.

Review is queued work. A change that takes ninety minutes to review does not get reviewed in a gap between meetings. It waits for a block of time, which may be tomorrow. A ten-minute review happens today. Latency is superlinear in size for this reason alone.

Conflict probability grows with time and surface area. A large pull request touches more files and stays open longer, and both increase the chance the base branch moves underneath it.

Reverting is proportionally risky. Reverting one focused change is safe. Reverting a change that also contained a rename, a dependency bump and a config change is a second risky operation.

The practical test is not a line count. It is: can a competent colleague hold this entire change in their head at once? If not, split it.

Scope: one reason to reject

A pull request should have a single reason a reviewer could reject it.

Mixing concerns makes review unresolvable. If a change contains a bug fix and an unrelated refactoring, and the reviewer likes the fix but disagrees with the refactoring, there is no verdict that expresses that. They must either block the fix or accept the refactoring.

Common mixtures worth separating:

Mixture	Why to split
Feature + reformatting	Formatting noise hides the logic change
Fix + refactor	Reviewer cannot accept one without the other
Behaviour change + dependency bump	Two different risk profiles
Rename + logic change	A rename is mechanically verifiable; logic is not
Multiple independent features	Each needs a different reviewer’s attention

Mechanical changes — renames, formatting, moving files — are the most valuable to separate. They are enormous in the diff and trivial to verify provided nothing else is mixed in. Once logic changes hide inside a 2,000-line rename, the reviewer has to read all of it.

Splitting work that seems indivisible

“This cannot be split” is usually a claim about the end state, not about the path.

Split by layer. Schema migration, then data access, then business logic, then the interface. Each merges independently and nothing is user-visible until the last.

Hide behind a flag. Merge incomplete work disabled. The code integrates continuously; the behaviour ships when ready. This is the standard technique in trunk-based development.

Separate the mechanical part. Do the rename or the move in its own pull request, merge it, then build on it.

Add before removing. Introduce the new path, migrate callers, remove the old one — three reviewable steps instead of one large swap.

Stack dependent changes. Open a pull request against a branch that is itself under review, so each piece is reviewed on its own. GitHub supports this by allowing any branch as the base, and it works well provided the stack is shallow — three or four deep before the bookkeeping outweighs the benefit. Merging the base rebases the dependent pull requests onto the new base automatically.

The description

The diff shows what changed. Only you can supply why.

Three questions, covered in PR Templates: what, why, and how to verify. Beyond that:

Say what you are unsure about. “I am not confident about the locking in flush()” directs attention where it is most valuable and is the single highest-value sentence in most descriptions.

Include evidence for anything visual or behavioural. A screenshot, a before-and-after measurement, the output of the command you ran. Not because it proves correctness, but because it demonstrates you checked.

Link the issue. Closes #42 connects the change to its reasoning permanently.

Call out what you deliberately did not do. “Not fixing the retry logic here — filed as #58” stops reviewers raising it and shows it was a decision rather than an oversight.

Commits within the pull request

Whether individual commits matter depends on your merge strategy.

If you squash merge, the branch’s commits vanish and only the squash message survives. Tidy intermediate commits are a courtesy to reviewers, not a permanent record.

If you merge or rebase, every commit lands on the main branch permanently, and their quality is your history’s quality. Here it is worth using interactive rebase to produce a clean sequence before review.

Either way, commits that separate mechanical from logical change make review much easier — a reviewer can skip the rename commit and concentrate on the one that matters.

Review velocity

Review latency is usually the dominant cost in a change’s lifetime, and it is mostly a cultural variable rather than a tooling one.

Review before starting new work. A pull request waiting on you is blocking someone else; your next task is not blocking anyone yet.

Agree an expectation, not a rule. “Reviews within one working day” gives people something to plan around. Unstated expectations mean everyone assumes someone else will do it.

Prefer a fast good-enough review to a slow perfect one. A day-old review lands while the author still has context. A three-day review means they have to reload the whole problem.

Distinguish blocking from optional. Prefix non-blocking comments — nit: is the common convention — so the author knows what must change.

Approve with minor comments. “Approving — please fix the typo before merging” is usually better than blocking. Trust is cheaper than another round trip.

Measuring it is straightforward and often revealing:

gh pr list --state merged --limit 50 --json number,createdAt,mergedAt,additions,deletions

What it doesLists recently merged pull requests with their creation and merge timestamps.

Why we run itCycle time is the metric that matters, and it is rarely what people assume. Feed it into a script to get the median rather than eyeballing it.

Expected resultA JSON array with number, createdAt and mergedAt for each pull request.

Plotting size against cycle time on your own repository is more persuasive than any general advice, because it is about your team.

Keeping pull requests current

A pull request open for a week is accumulating conflict risk and losing context.

Update the branch when the base moves, particularly if up-to-date branches are required:

gh pr view PULL_NUMBER --json mergeStateStatus --jq .mergeStateStatus
gh pr update-branch PULL_NUMBER

If the same pull request needs updating repeatedly, that is a signal the change is too large or has been open too long — not a reason to automate the updating.

Merge strategy

Three options, three histories. Decide once per repository rather than per pull request.

Strategy	Best when
Squash	Pull requests are the unit of change; intermediate commits are working notes
Merge commit	Individual commits are meaningful and worth preserving
Rebase	You want linear history and each commit stands alone

Squash is the common default because it makes main a clean sequence of changes, one per pull request, each with an obvious revert. It discards intermediate history, which is a real loss on carefully-crafted commit sequences. The trade-offs are covered in Squash Merging and Rebase and Merge.

Whichever you choose, enable automatic branch deletion on merge. Stale branches accumulate quickly and make the branch list useless.

Generated files and dependencies

Mark generated files as generated. A .gitattributes entry with linguist-generated=true collapses them in the diff, which keeps the human-written change visible:

package-lock.json    linguist-generated=true
*.pb.go              linguist-generated=true

Keep dependency updates separate. A dependency bump has a different risk profile and a different reviewer. Mixed into a feature change, it gets no scrutiny at all.

Review new dependencies properly. Adding a dependency is a permanent commitment to someone else’s maintenance. Who maintains it, when was it last released, what does it pull in transitively, and does it need to exist?

Security review

Some changes deserve specific attention regardless of size:

Authentication, authorisation and session handling
Anything parsing untrusted input
Cryptography — particularly anything hand-rolled
CI workflow files, which run with repository credentials
New dependencies and permission changes
Anything that logs, in case it logs a secret

For contributions from forks, remember CI restrictions exist because the code is untrusted, and a fork’s change to a workflow file deserves particular care.

Measuring your own repository

General advice is weaker than your own data, and the data is a few commands away.

Cycle time by size:

gh pr list --state merged --limit 100 \
  --json number,additions,deletions,createdAt,mergedAt \
  --jq '.[] | {
    size: (.additions + .deletions),
    hours: (((.mergedAt | fromdateiso8601) - (.createdAt | fromdateiso8601)) / 3600 | floor)
  }' \
| jq -s 'group_by(if .size < 100 then "small" elif .size < 500 then "medium" else "large" end)
         | map({bucket: (.[0] | if .size < 100 then "small" elif .size < 500 then "medium" else "large" end),
                count: length,
                median_hours: (sort_by(.hours) | .[length / 2 | floor] | .hours)})'

Output:

[
  { "bucket": "large",  "count": 12, "median_hours": 71 },
  { "bucket": "medium", "count": 34, "median_hours": 19 },
  { "bucket": "small",  "count": 54, "median_hours": 4 }
]

That relationship holds on nearly every repository, and seeing it in your own numbers is far more persuasive than any general claim about ideal sizes. Note also that it is not linear — large pull requests are disproportionately slow, because they wait for a block of time rather than fitting into a gap.

Time to first review, which is usually the dominant component:

gh pr list --state merged --limit 50 --json number,createdAt,reviews \
  --jq '.[] | select(.reviews | length > 0)
            | (((.reviews[0].submittedAt | fromdateiso8601) - (.createdAt | fromdateiso8601)) / 3600 | floor)' \
| jq -s 'sort | {p50: .[length/2|floor], p90: .[length*0.9|floor], max: .[-1]}'

If the median is under four hours and the ninetieth percentile is three days, you do not have a review speed problem — you have a small number of pull requests nobody wants to review. Those are usually the large ones, which returns to the same root cause.

Splitting a change that is already too large

Sometimes the pull request exists and is too big. Three ways out, in order of preference.

Extract the mechanical part. Open a second pull request containing only the rename, the move or the reformat, merge it, and rebase the original. The remaining diff is often a fraction of the size.

git switch -c extract-rename main
git checkout my-big-branch -- src/renamed_module.py
git commit -m "Rename storage module ahead of the retry work"
gh pr create --fill

Split by commit. If the history is clean, cherry-pick the independent commits onto a new branch:

git switch -c first-half main
git cherry-pick a3f8c21 b7e2d94
gh pr create --fill

Stack it. Keep the work as is, but layer it — covered in Draft Pull Requests.

What does not work is asking a reviewer to “just review the first three files”. Review is about the change as a whole, and a partial review of a whole change is the rubber stamp under another name.

Conventions worth agreeing as a team

Most of this lesson is judgement. A few things benefit from being written down once, because inconsistency costs more than whichever option you pick.

Decision	Why write it down
Merge strategy	Determines what `main` looks like permanently
Expected review turnaround	Unstated means everyone assumes someone else
Blocking versus non-blocking comment convention	`nit:` needs to mean the same to everyone
Whether draft means “do not review”	Otherwise it means nothing
When a second reviewer is required	Better decided calmly than per pull request
Who may merge	Author, reviewer, or either
What must be linked	An issue, a ticket, nothing

CONTRIBUTING.md is the right place, and short is better. Seven lines that people have read beat three pages nobody has.

Pull requests as a historical record

A merged pull request is the most complete explanation of a change that will ever exist. The diff shows what; the description shows why; the review shows what alternatives were considered and rejected.

That makes a few habits worth more than they seem in the moment:

Write the description for a stranger in two years. That stranger is often you.

Record rejected alternatives. “We considered doing this in the client, but that would have meant duplicating the retry logic in three places” prevents someone re-proposing it.

Link the issue. It is the chain from git blame to a line, to the commit, to the pull request, to the reasoning.

Keep discussion in the pull request. A decision reached in chat and merged silently is a decision nobody can reconstruct. A one-line summary of the conversation costs nothing.

The archaeology only works if the links exist. Every unlinked merge is a piece of reasoning that has to be guessed at later.

Anti-patterns

The mega-PR. Two thousand lines, four concerns, three days of review, approved unread.

The drive-by refactor. A genuine fix wrapped in unrelated restructuring.

The rubber stamp. Approving without reading, which makes required review theatre.

Perfection blocking. Refusing to approve over preference, until people route around review.

The zombie PR. Open for six weeks, conflicts daily, nobody willing to close it.

Force-pushing mid-review. Detaches comments and forces re-reading.

Reviewing only the diff. Some problems only appear when the code runs.

Merging your own unreviewed change because it is urgent. Sometimes correct — and it should be visible, deliberate and rare, not a habit.

A checklist worth using

Before requesting review:

One sentence, no “and”, describes the change.
You have read your own diff and removed the debug statements.
The description says what, why, and how to verify.
You have flagged anything you are unsure about.
Checks pass.
The branch is based on current main.
Generated files are marked or separated.
The reviewer you requested is the right person.

Practices by team size

What is right depends on how many people are involved, and advice that ignores that is advice that will not fit.

Solo. Pull requests are still worth using — for the record, the CI gate, and the diff review. Skip approvals, which you cannot give yourself. The main benefit is archaeology, not review.

Two to five. Review everything, quickly. Latency is the dominant cost at this size and it is entirely within your control. Formal process adds little; a shared expectation about turnaround adds a lot.

Five to twenty. Ownership stops being obvious, so CODEOWNERS starts earning its place. Review load needs watching — one person becoming the default reviewer is the common failure.

Twenty or more. Governance becomes structural rather than social: rulesets, required checks, and probably a merge queue. The failure mode shifts from “nobody reviewed it” to “review became theatre”, and the answer to that is smaller pull requests rather than more requirements.

Public contributions. Everything above, plus explicit guidelines, because contributors have no context and no way to acquire it except what you write down.

The one thing to take away

If a team adopts one practice from this lesson, it should be smaller pull requests.

It improves review quality, because reviewers can hold the whole change in their head. It improves review latency, because a ten-minute review fits into a gap and a ninety-minute one does not. It reduces conflicts, because branches live for hours rather than weeks. It makes reverts safe, because a focused change can be undone without collateral damage. And it makes every other practice here easier — descriptions are simpler to write, reviews are simpler to respond to, and the merge strategy matters less.

Nothing else on this page has that breadth of effect. Most of the rest is downstream of it.

What you learned

Almost every pull request problem is a size problem in disguise.
The size test is whether a colleague can hold the whole change in their head — not a line count.
A pull request should have exactly one reason it could be rejected.
Work that seems indivisible usually splits by layer, behind a flag, or by separating mechanical from logical change.
Review latency is mostly cultural, and reviewing before starting new work is the highest-leverage habit.
Rewriting history mid-review costs reviewers their context; add commits instead.
Merge strategy is a repository decision about what your history should look like.

Enforcing size, gently

Size is the recurring theme of this lesson, and it can be surfaced automatically without blocking anyone.

name: Size
on: pull_request

permissions:
  pull-requests: write

jobs:
  label:
    runs-on: ubuntu-latest
    steps:
      - env:
          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          PR: ${{ github.event.pull_request.number }}
        run: |
          set -euo pipefail
          changed=$(gh pr view "$PR" --json additions,deletions \
                     --jq '.additions + .deletions')
          if   [ "$changed" -lt 100 ]; then size="size/S"
          elif [ "$changed" -lt 500 ]; then size="size/M"
          else                              size="size/L"
          fi
          gh pr edit "$PR" --add-label "$size"
          [ "$size" = "size/L" ] && \
            echo "::notice::This pull request changes $changed lines. Consider splitting it."
          exit 0

Labelling rather than blocking is deliberate. A hard limit produces two behaviours, both bad: people split changes artificially along lines that make review harder, and people add exclusions until the rule means nothing.

A label makes size visible in the pull request list, which lets reviewers plan — and it gives you the data for the cycle-time analysis earlier in this lesson without any extra work.

Generated files should be excluded from the count, or every dependency update is size/L. Filtering on the file list before summing handles it.

GitHub CLINext cluster: operating all of this from the terminal.

Short-Lived BranchesThe branching discipline that keeps pull requests small.

Interactive RebaseTidying a commit sequence before review.

Squash MergingWhat squash actually does to your history.

Pull Request Automation with the GitHub APIList, create, update, review and merge pull requests through the API

gh pr: The Pull Request Lifecycle from the TerminalCreate, list, check out, review, watch checks and merge pull requests with gh

Professional ToolkitCODEOWNERS, pull request and issue templates, and repository configuration checklists ready to adapt.