Pull requests & code review

On almost every team, code reaches main through a pull request (PR): a proposal to merge one branch into another, with a place for discussion, automated checks and approval. Git itself has no pull requests; they are a feature of hosting platforms like GitHub (GitLab calls them merge requests). This lesson covers the full life cycle on GitHub — preparing a branch, writing a PR people want to review, handling feedback, reviewing others' work, and choosing how to merge.

#What a pull request is

A PR compares two branches on the server:

• The base branch receives the changes, usually main.
• The head branch contains them, for example feature/search.

GitHub shows the commits and the combined diff of head against the point where it left base — the same thing you get locally with git diff main...feature/search (three dots: "changes on the branch since it diverged"). The PR is live: push more commits to the head branch and they appear in it automatically. Nothing reaches main until someone presses merge.

main            A ── B ── E
                      \
feature/search         C ── D        ← PR #42: feature/search → main

Before the merge, people discuss and review the change, automated checks (tests, linters, builds) run against it, and repository rules decide whether it may be merged at all.

#Preparing a branch worth reviewing

Start from an up-to-date base. Create your branch from the latest main so the diff contains only your work:

git switch main
git pull
git switch -c feature/search-notes

Keep it focused and small. One PR should do one thing: a feature, a bug fix, or a refactor — not all three. Reviewers' attention drops sharply on large diffs; a few hundred changed lines is a sensible upper target. If a feature is big, land it as a sequence of smaller PRs (for example, the data model first, then the API, then the UI).

Review your own diff first. Before anyone else sees it, read exactly what you are proposing:

git fetch
git log --oneline origin/main..HEAD     # the commits in your PR
git diff --stat origin/main...HEAD      # files touched
git diff origin/main...HEAD             # every line

You will catch leftover debug logs and accidental file changes. Tidy messy commits with an interactive rebase, then push with an upstream:

git push -u origin feature/search-notes

#Opening the pull request

After a push, GitHub prints a link and shows a "Compare & pull request" banner on the repository page. Or create it from the terminal with GitHub's CLI:

gh pr create --base main --title "Add search to the notes list" --body-file pr-body.txt --reviewer alice

Creating pull request for feature/search-notes into main in ada/notes-app

https://github.com/ada/notes-app/pull/42

Add --draft to open a draft PR: visible and able to run checks, but clearly marked as not ready, so nobody reviews it prematurely. Mark it ready when it is.

#Writing the description

The title should read like a good commit summary: specific and imperative. "Add search to the notes list" beats "Search stuff". The description gives the reviewer the context the diff cannot:

What:
Adds a search box above the notes list that filters notes by title
and body as you type.

Why:
Users with large notebooks can't find anything (#118). This is the
smallest useful version; server-side search can follow if needed.

How to test:
1. Run `npm run dev` and create a few notes.
2. Type in the search box: the list filters immediately.
3. Clear the box: all notes return.

Notes for reviewers:
Filtering lives in `useNoteSearch.ts`. I debounced input by 150 ms;
happy to discuss the value.

Closes #118

Writing Closes #118 (or Fixes, Resolves) links the issue and closes it automatically when the PR merges into the default branch. Screenshots or a short recording help enormously for UI changes. Many repositories provide a template at .github/pull_request_template.md that pre-fills this structure.

#Checks, rules and updating the PR

Most teams configure status checks: CI workflows that run the tests and linters on every push to a PR, shown as green ticks or red crosses. With gh you can watch them from the terminal:

gh pr checks

Repository owners usually protect main with branch protection rules (or rulesets): require passing checks, require one or more approving reviews, forbid direct pushes and force pushes. If the merge button is disabled, the PR page tells you which rule is not satisfied yet.

#Responding to feedback

When a reviewer asks for changes, make them on the same branch and push; the PR updates itself. Two styles are common:

• Add follow-up commits ("Handle empty search query"). Reviewers can see exactly what changed since their last look. This is the simplest default, especially if the PR will be squash-merged.
• Rewrite history cleanly: commit with git commit --fixup=<sha>, then git rebase -i --autosquash origin/main and git push --force-with-lease. This keeps a tidy series, at the cost of making "what changed since my review" harder to see. Only do it on a branch that is yours alone.

Reply to every comment, even with "Done" or a short explanation of why you disagree. Then re-request review.

#Keeping up with main

If main moves on while your PR is open, you may need to bring the branch up to date, either because of conflicts or because a rule requires it. Either merge main in (git merge origin/main, then push — no force needed) or rebase (git rebase origin/main, then git push --force-with-lease). Follow your team's convention. GitHub's "Update branch" button does the same on the server.

#Reviewing someone else's pull request

An approval says "I have read this and I am comfortable with it reaching main", so take it seriously.

#What to look at, in order

1. Purpose. Read the description. Does the change solve the stated problem, and is it the right approach? Raise design concerns first, before polishing details that may be rewritten.
2. Correctness. Edge cases, error handling, empty and very large inputs, concurrency, security (untrusted input, secrets, permissions).
3. Tests. Is the new behaviour tested, and would the tests fail if the code were wrong?
4. Readability. Clear names, reasonable structure, comments where intent is not obvious.
5. Style. Last — and ideally left to formatters and linters entirely.

For anything non-trivial, run the code. Check out the PR locally in one command:

gh pr checkout 42

Without gh, GitHub exposes every PR as a ref you can fetch: git fetch origin pull/42/head:pr-42 then git switch pr-42.

#Writing comments that help

Comment on specific lines, and make each comment's weight clear. Compare:

Ask questions rather than issue verdicts, explain the why, and label minor points nit:. GitHub also lets you attach a suggested change to a line comment (the "add a suggestion" button); the author can apply it with one click. Praise good work too — it tells people what to keep doing.

When you finish, submit the review as one of three types: Comment (feedback, no verdict), Approve, or Request changes (blocking until you re-review).

#Merging

GitHub offers three merge buttons. The repository settings decide which are allowed.

Create a merge commit — every commit from the branch plus a merge commit (like git merge --no-ff):

A ── B ── E ──────── M   ← main
      \             /
       C ───────── D

Squash and merge — all the branch's changes become a single new commit on main:

A ── B ── E ── S   ← main      (S contains C + D; C and D are not on main)

Rebase and merge — the branch's commits are replayed onto main individually, without a merge commit:

A ── B ── E ── C' ── D'   ← main

Squash is popular because work-in-progress commits never reach main and each PR becomes one revertable commit. Merge commits preserve the full branch history. Rebase-and-merge gives linear history while keeping each commit, so it suits PRs whose commits were carefully curated. Which one your team uses is a workflow decision, discussed in the next lesson.

After merging, delete the branch (GitHub offers a button, or gh pr merge 42 --squash --delete-branch), and tidy up locally:

git switch main
git pull
git fetch --prune
git branch -d feature/search-notes

#Practice

Take a change you would like to make to any repository you own on GitHub (or create a small one).

1. Create a branch from an up-to-date main, make a focused change in two or more commits, and review your own diff against origin/main.
2. Push it and open a draft PR with a What / Why / How to test description that closes an issue you created for it.
3. Leave one self-review comment, mark the PR ready, and merge it with "Squash and merge".
4. Clean up locally and confirm the branch is gone everywhere.

git switch main && git pull
git switch -c docs/install-steps
# ...edit README.md, commit twice...
git fetch
git log --oneline origin/main..HEAD
git diff origin/main...HEAD

git push -u origin docs/install-steps
gh issue create --title "README lacks install steps" --body "New contributors can't set up the project."
# suppose it was issue #7
gh pr create --draft --base main --title "Document installation steps" \
  --body "What: adds install steps to the README.

Why: new contributors could not get started.

How to test: follow the steps on a clean machine.

Closes #7"

gh pr ready
gh pr merge --squash --delete-branch
git switch main
git pull
git fetch --prune
git branch -D docs/install-steps     # -D: the squash commit is new, so -d may refuse
git branch -a                        # the branch is gone locally and remotely

#Key takeaways

• A PR proposes merging a head branch into a base branch, combining discussion, CI checks and approval rules.
• Keep PRs small and focused, start from an up-to-date main, and review your own diff before asking others.
• A good description explains what, why and how to test, and links issues with Closes #n.
• Review purpose and correctness before style; make comments specific, kind and clearly blocking or not.
• Merge commit, squash and rebase-and-merge produce different histories; squash creates one new commit, so delete the local branch with -D.