backend / git / 11_pr_workflow.md

Pull Request Workflow

6 interview angles 6 min read source

Pull Request Workflow

A PR (or “merge request” in GitLab) is a proposal to merge a branch into another, plus the review/CI machinery around it. The git mechanics are simple; the social/process layer is where teams differ.

The basic flow

git switch -c feature/jira-123-add-user
# ... commit changes ...
git push -u origin feature/jira-123-add-user
# open PR on GitHub/GitLab → review → merge

-u (or --set-upstream) tracks the remote branch so future git push and git pull work without arguments.

Three merge button options on GitHub

Option Result on main When to use
Create a merge commit branch commits + a merge commit feature has meaningful sub-commits worth preserving
Squash and merge one commit on main with PR title as message most teams’ default for clean history
Rebase and merge branch commits replayed on main, no merge commit linear history, preserves per-commit story

Most modern teams pick squash and merge for application code (one commit per PR keeps git log main readable) and rebase merge for libraries where per-commit history matters.

See 02_merge_vs_rebase.md for the underlying mechanics.

Keeping the PR branch up to date

Two ways to incorporate latest main:

git switch feature
git fetch origin
git rebase origin/main             # linear, rewrites your branch
# or
git merge origin/main              # adds a merge commit

If the branch is shared (others might have pulled it), use merge. If it’s just yours, rebase.

After rebase you’ll need git push --force-with-lease (because rewritten history). --force-with-lease is safer than --force — it refuses to push if someone pushed to the branch since your last fetch.

git push --force-with-lease

Never plain --force on shared branches.

Sizing a PR

The single biggest review-quality lever:

PR size Review quality
< 100 lines diff thorough
100–500 lines OK if focused
500–1000 spotty, lots of “LGTM” without real reading
> 1000 lines reviewers skim, real bugs slip through

Studies (Google, SmartBear, etc.) consistently show review effectiveness drops sharply past ~400 lines. Split big PRs into stacked smaller ones if your tooling supports it (Graphite, Sapling, ghstack).

Stacked PRs

When you have feature work that’s naturally a sequence:

main ──── PR1: schema migration ──── PR2: API endpoint ──── PR3: UI integration

Each PR is small and focused. Review proceeds in parallel. Tools (Graphite, Sapling) automate the rebases when an upstream PR merges.

Without tooling: open PR2 against PR1’s branch (not main); when PR1 merges, retarget PR2 to main and rebase.

Draft / WIP PRs

A draft PR (GitHub: “Draft”, GitLab: “WIP:”) doesn’t trigger review-required state but does trigger CI. Use for:

  • Early feedback on direction before finishing.
  • Showing CI passing for in-progress work.
  • Parking work over a weekend visibly.

Convert to ready-for-review when done. Avoid leaving drafts open for weeks — they’re branches in disguise.

PR description: the actual important part

A bad PR title + empty body is the leading cause of “what does this even do?” reviews. Minimum:

## Summary
What this changes and why.

## Changes
- bullet points of meaningful changes

## How to test
- steps to reproduce
- screenshots if UI

## Related
- closes #123
- depends on #456

closes #123 auto-closes the issue when the PR merges. Tooling cares about this; humans care about the summary.

Code review etiquette (the social part)

For reviewers:

  • Be specific. “This is wrong” → “This if clause runs when x is None, but the function above already returned in that case — dead code.”
  • Distinguish blocking from non-blocking. “Nit:” / “Suggestion:” / “Question:” prefixes help.
  • Approve when it’s good enough; don’t gate on personal preferences.

For PR authors:

  • Respond to every comment, even “OK, fixed in .”
  • Don’t force-push between rounds of review (it loses the ability to see “what’s changed since I last reviewed”). GitHub’s “compare” view degrades after force-push.
  • Either rebase + force-push at the end, or use “fixup” commits during review and squash on merge.

Merge conflicts

When main moves while your PR is open:

git fetch origin
git rebase origin/main
# resolve conflicts per commit
git push --force-with-lease

GitHub also lets you resolve simple conflicts in the web UI. For real conflicts, do it locally — better tooling, faster.

Heavy conflict means the PR is too big or has lived too long. See 10_branch_strategies.md.

Required CI checks

Branch protection should require CI to pass before merge. Common checks:

  • Build passes (compiles, no import errors).
  • Unit tests pass.
  • Linter clean.
  • Type-checker clean (mypy / pyright).
  • Security scan (Snyk, Trivy).
  • Coverage threshold met (or at least not regressed).

Faster CI = faster reviews. A 30-minute test suite means PRs sit waiting; a 3-minute suite means PRs ship the same day.

Preview/review environments

For services, deploy each PR to a temporary URL (pr-123.staging.example.com). Reviewers can click and try. Standard in:

  • Vercel / Netlify (frontend)
  • AWS Amplify
  • Heroku Review Apps
  • Render preview environments
  • Custom k8s with PR-namespace deploys

Pays off when reviewers can’t easily run the change locally.

Closing without merging

When the PR is wrong or superseded:

  • Close without merging — branch can stay or be deleted.
  • Keep the PR open as a record (don’t delete) — git log main won’t show it, but the discussion is preserved.

Don’t merge “to be polite” — the wrong code is worse than no code.

After merge

  • Delete the branch (GitHub does this automatically if configured).
  • Pull main locally:
    git switch main
    git pull
    git branch -d feature/jira-123-add-user
  • Verify deploy (if continuous deployment).
  • Update the linked issue/ticket.

Common interview confusions

  • “Squash merge loses the commits.” — they’re discarded from main, but the original branch (and PR) keeps them visible. After branch deletion, only the GitHub PR record retains them.
  • “Force-push is always bad.”--force-with-lease on a branch you own is fine. Plain --force on a shared branch wipes other people’s work.
  • “GitHub’s Update branch button rebases.” — by default it merges main into your branch (creates a merge commit). To rebase, do it locally and force-push.

Interview angle

  • “Walk me through your PR workflow.” — branch from main, commit, push, open PR with description, address review, CI passes, merge (squash/rebase/merge), delete branch.
  • “Squash merge vs merge commit vs rebase merge — when which?” — squash for clean main with one commit per feature; merge commit for preserving the history of a branch (libraries, big features); rebase merge for linear history while keeping per-commit story.
  • “How do you keep a long-running PR up to date with main?” — rebase onto origin/main, force-push-with-lease. Or merge main in if the branch is shared.
  • “Why is --force-with-lease better than --force?” — it refuses if the remote moved since your last fetch (someone else pushed). Plain --force overwrites their work silently.
  • “What’s a stacked PR and why?” — a PR built on top of another PR’s branch instead of main. Lets you split a big change into a sequence of small reviewable PRs without waiting for each to merge first.
  • “What makes a good PR description?” — summary (what + why), bullet points of changes, test instructions, links to issues. Empty descriptions slow down review and break tickets/auditing.