1. Courses
  2. Developer Foundations
  3. GitHub for Teams
  4. Stop 2

35 minIntermediate

Pull requests and code review

A pull request is where your team decides, together, that a change is safe to ship. This lesson shows what a PR is made of, how to write one that gets a fast yes, how to review well, what each merge button does to history, and how a PM can read a PR in ten minutes without reading every line.

By the end you can

  • Name every part of a PR page and say what each one tells you
  • Write a PR description and a template that reviewers can act on
  • Review with line comments, suggestions, approve and request changes
  • Set up CODEOWNERS so the right people are asked automatically
  • Pick merge commit, squash or rebase, knowing what each leaves on main

The everyday picture

Before a festival rush, the pricing team prepares a new price list for 320 stores. One person types it. A second person checks it line by line against the approved offer sheet before it goes out. If she finds "Fresh milk 1L: $0.15" instead of $1.50, she circles it and sends it back. The typist fixes it, she checks again, signs it, and only then does it reach the stores.

Nobody thinks the second check is an insult. It is how a chain avoids selling milk at a tenth of the price for a weekend. A pull request is that second check for code. The branch is the draft price list, the reviewer is the second person, the automated checks are the calculator that adds up every column, and "merge" is the moment the list goes to the stores.

What a pull request is made of

A (PR) asks to merge one branch into another. The branch with your work is the . The branch you want to merge into, usually main, is the . A PR is not a copy of your code. It points at your branch, so every new commit you push to that branch appears in the PR at once and the checks run again.

The PR page has four tabs, and each one answers a different question:

  • Conversation: why this change exists. The title, the description, linked issues, review comments, and a timeline of everything that happened.
  • Commits: the commits on the head branch, in order. Good commit messages (see Everyday Git) make this tab readable.
  • Checks: the results of automated jobs such as lint, tests and build. These come from GitHub Actions (see GitHub Actions: continuous integration) or other tools, and each one is a .
  • Files changed: the diff, file by file. This is what the code review happens on.

One detail explains many confusing diffs. The PR shows the changes on the head branch since it split from the base, measured from their common ancestor, the (see Branches and merging). New commits on main do not appear as "removed" lines in your PR. They only matter when they conflict with yours (see Merge conflicts without panic).

Quick check

A reviewer asked for a fix, and you pushed two more commits to your branch. What do you need to do so the PR shows them?

Open pull request draft while unfinished ready for review Checks run lint, test, build all green Code review comments, suggestions Changes requested author fixes Push a fix checks run again approve Merge merge, squash, rebase Branch rules 1 approval + green after merge Issue closes head branch deleted
The PR lifecycle. Every push re-runs the checks, so "request changes" loops back through a fix and a new green run. Merge is allowed only when the branch rules are met (see Branching strategies, releases and versions); then a linked issue closes and the branch can be deleted.

Writing a PR that gets a fast yes

Reviewers are busy. Your job as the author is to make "yes" easy and "no" obvious. Three habits do most of the work.

Keep it small. A PR that changes 80 lines gets read carefully. One that changes 1,500 lines gets skimmed and approved with "LGTM" (looks good to me). Many teams aim for under roughly 400 changed lines per PR, not counting lockfiles and generated code. Split big work into a chain: first the database change, then the API, then the screen. Each piece can be reviewed and merged on its own.

Write the description for a stranger. Say what changed and why, how you tested it, what the reviewer should look at first, and what could go wrong. Link the issue with a such as Fixes #123, so the issue closes when the PR merges into the default branch (see Planning on GitHub). For anything a user can see, paste a screenshot or a short screen recording. A reviewer who can see the new Partner app receipt does not have to imagine it.

Use a draft while the work is unfinished. A cannot be merged, and code owners are not asked to review it automatically. It is a good way to show early work, get CI running, and ask one question without asking for a full review. Click Ready for review when you are done.

Finally, review your own PR before anyone else does. Open the Files changed tab and read it as a reviewer would. You will find the leftover console.log, the file you did not mean to commit, and the test you forgot.

Quick check

Which PR description lets a reviewer start in under a minute?

Reviewing: comments, suggestions and the three verdicts

In the Files changed tab you can comment on one line, or drag across several lines to comment on a block. Click the suggestion button in the comment box and GitHub inserts a suggestion code block with the selected lines, which you edit into what you want. The author (or anyone with write access) can then press Commit suggestion, or Add suggestion to batch to apply several as one commit. Suggestions turn "please rename this" into a one-click fix.

If you click Start a review instead of commenting right away, your comments stay pending and only you can see them. This lets you read the whole PR first and delete comments that the next file answers. Tick Viewed on each file you finish; it collapses, and if the author changes it later it unticks itself. When you are done, open Review changes, write a short summary, and pick one verdict:

  • Comment: feedback only, no decision.
  • Approve: you are happy for this to merge.
  • Request changes: something must be fixed first. When branch rules require reviews, this blocks the merge until the reviewer approves or someone with write access dismisses the review.

When the author has dealt with a comment, the thread is closed with Resolve conversation. Teams can require every thread to be resolved before merging (see Branching strategies, releases and versions), which stops "I'll do it later" comments from disappearing.

Good review comments are specific and kind, and they say how important they are. A useful habit is a label at the start: nit: for a small style point that should not block, question: when you want to understand, blocking: when it must change before merge. Comment on the code, never the person ("this query runs once per sale" rather than "you wrote a slow query"). And agree as a team on response time, for example a first review within one working day. A PR waiting three days for review is three days of risk that it goes stale and conflicts.

Think first: you reviewed a PR, approved it, and the author then pushed two more commits. Does your approval still count?

By default, yes. GitHub keeps the approval even though you never saw the new code. That is why rulesets have options such as "Dismiss stale pull request approvals when new commits are pushed" and "Require approval of the most recent reviewable push". With either one on, the new commits need a fresh approval. You will set these in Branching strategies, releases and versions.

CODEOWNERS: asking the right people automatically

A CODEOWNERS file maps paths to the people or teams who own them. When a PR touches an owned path, GitHub requests a review from those owners. GitHub looks for it in .github/, then the repository root, then docs/, and uses the first one it finds. It reads the file from the PR's base branch, so a PR cannot change its own reviewers by editing CODEOWNERS.

Each line is a pattern (gitignore style) followed by owners. The last matching line wins, so put the general rule first and the specific ones below it. Owners must have write access to the repository, or the line is ignored. If the ruleset option "Require review from Code Owners" is on, an approval from any one of the listed owners for each changed path is enough. Treat it as a routing table: it makes sure the payment team sees payment code, without making every PR wait for everyone.

Quick check

A developer's PR edits .github/CODEOWNERS to remove the payments team from /src/payments/, and also changes a payment file. Who is asked to review the payment file?

Merge commit, squash or rebase: what lands on main

The green merge button has three modes, and each one leaves a different history on main. Repository admins choose which are allowed in Settings, under "Pull Requests": Allow merge commits, Allow squash merging and Allow rebase merging.

  • Create a merge commit runs a merge with --no-ff (see Branches and merging). All the PR's commits stay, and a new merge commit with two parents ties them in. You keep the true history, including "fix typo".
  • Squash and merge combines all the PR's commits into one new commit on main. One PR becomes one commit, which is easy to read, to revert, and to bisect (see Reading code like a detective). The cost is that the individual commits are gone from main. GitHub fills in the commit message from the PR title and commits, and admins can change that default.
  • Rebase and merge replays each commit onto the tip of main without a merge commit (see Rewriting history safely). History is linear and every commit is kept, but each one gets a new ID because its parent changed (see Inside the .git folder), and GitHub always records itself as the committer.

For most product teams, squash is the simplest default: one PR, one commit, a clean main, and Conventional Commit titles that a changelog tool can read (see Branching strategies, releases and versions). Teams with carefully crafted commits sometimes prefer rebase. One trap with squash: if you keep working on the same branch after it was squashed, your next PR will contain commits that are already on main in squashed form, and you get conflicts. Delete the branch after merging (turn on Automatically delete head branches) and start the next piece of work from a fresh main.

Quick check

A team writes careful, small commits and wants every one of them on main, but no merge commits. Which button fits?

Before: PR with A, B, C 1 2 3 A B C main feature Merge commit all commits kept 1 2 3 A B C M 2 parents Squash and merge one new commit 1 2 3 S S = A + B + C Rebase and merge copies with new IDs 1 2 3 A' B' C'
One PR, three results. A merge commit keeps A, B and C and adds M with two parents. Squash adds one new commit S that contains all three changes. Rebase copies A, B and C onto the tip of main as new commits with new IDs.
Merge methodGood forWatch out
Merge commitFull, true history; long-lived branches such as a release branchNoisy main with "wip" commits; harder to read and bisect
Squash and mergeOne PR = one commit; easy revert; clean changelog from PR titlesLoses individual commits; do not keep working on a squashed branch
Rebase and mergeLinear history that keeps well-made commitsNew commit IDs; every commit must pass tests on its own

How a PM reads a PR without reading every line

You do not need to review the code to review the change. Work through the page in this order and you will catch most product problems in about ten minutes:

  1. Title and linked issue. Does the title match what was asked? Open the issue and compare the acceptance criteria.
  2. Description. Is there a "why", a test plan and a risk note? No description is itself a finding.
  3. Screenshots. For any screen change, check the copy, the text in every language the app ships in, empty states and error messages.
  4. Checks. Green, or are any skipped or failing?
  5. The file list, not the code. Look for risky files: database migrations and schema.prisma, .github/workflows/, package.json and the lockfile, environment or config files, anything under payments, auth or pricing. Then check that test files changed too.
  6. Size. Thirty files for a copy change means something else is riding along.

Ask your questions as review comments, so the answers stay with the code. Unless your team agrees otherwise, use Comment rather than Approve: an approval counts toward the merge rule, and it should come from someone who has read the code.

Two habits make this faster. Use gh pr diff 42 --name-only to see only the file list from the terminal, and keep a saved search such as is:pr is:open label:enhancement for the PRs that touch your product. The Practice section below gives you a real-looking PR to read this way.

Build it

Add a PR template and CODEOWNERS to partner-api, then open, review and merge a PR from the terminal with the GitHub CLI (gh, set up in GitHub: remotes, clone, push, pull and authentication). The commands are the same in Git Bash, PowerShell and zsh. The Practice section has a browser exercise on closing keywords, the part of the description that links a PR to its issue.

  1. Add a pull request template

    GitHub fills every new PR description with this file. It must be merged into the default branch before it takes effect. The HTML comments are hints for the author and do not show in the rendered description.

    Markdown
    <!-- .github/pull_request_template.md -->
## What and why
<!-- One or two sentences. Link the issue: "Fixes #123" closes it on merge. -->
Fixes #

## How I tested it
- [ ] Unit tests added or updated
- [ ] Ran it locally against the staging database
<!-- Paste commands or steps a reviewer can repeat. -->

## Screenshots
<!-- Required for any screen change (Store app, Partner app, dashboard). -->

## Risk and rollout
- Touches: <!-- e.g. pricing, payments, migrations, CI -->
- Safe to deploy during trading hours? yes / no
- How to roll back:

## Reviewer notes
<!-- Where to start reading, anything you are unsure about. -->

  2. Add CODEOWNERS

    Replace your-org with your GitHub organisation and use real team names. General rule first, specific rules after, because the last match wins. Owning /.github/ means changes to workflows and to CODEOWNERS itself need a lead's eyes.

    Output
    # .github/CODEOWNERS
# Default: the backend team reviews everything.
*                         @your-org/backend

# Database schema and migrations: backend leads.
/prisma/                  @your-org/backend-leads

# Pricing and payments: their owners, plus the leads.
/src/pricing/             @your-org/pricing @your-org/backend-leads
/src/payments/            @your-org/payments @your-org/backend-leads

# CI workflows, templates and this file: platform team.
/.github/                 @your-org/platform

    Commit both files on a branch and merge them like any other change. Then open .github/CODEOWNERS on GitHub: lines with errors (for example a team without write access) are highlighted there.

  3. Open a draft PR from the terminal

    Push your branch, then create the PR. --draft keeps code owners from being pinged until you are ready. --body-file reads the description from a file, so you can write it in VS Code.

    Terminal
    git switch -c feature/festival-offer
# ...edit, commit with Conventional Commit messages...
git push -u origin feature/festival-offer
gh pr create --draft --base main --title "feat(discount): festival offer at 10% for partner shops" --body-file pr-body.md --label enhancement

    gh prints a line like the one below and then the PR's URL. Your organisation and PR number will differ.

    Output
    Creating draft pull request for feature/festival-offer into main in your-org/partner-api
    Terminal
    gh pr checks --watch      # wait for CI on your branch
gh pr ready               # mark it ready: code owners are now requested
gh pr view --web          # open it in the browser to add screenshots

  4. Review someone else's PR

    Check out the PR locally to run it, look at the file list first, then submit a verdict. Use the browser for line comments and suggestions; use the CLI for the verdict when that is quicker.

    Terminal
    gh pr list --search "review-requested:@me"   # PRs waiting for you
gh pr checkout 42                            # creates a local branch for PR 42
gh pr diff 42 --name-only                    # only the file list
npm test                                     # run it yourself if in doubt
gh pr review 42 --request-changes --body "blocking: the migration locks the sale table; please add the index CONCURRENTLY"
gh pr review 42 --approve --body "Tested locally with the festival offer sheet. Thanks!"

  5. Merge with squash and clean up

    --squash picks the method, and --delete-branch removes the branch locally and on GitHub. If the checks are still running, --auto merges as soon as all the rules are met (auto-merge must be enabled in the repository settings).

    Terminal
    gh pr merge 42 --squash --delete-branch
# or, while checks are still running:
gh pr merge 42 --squash --auto --delete-branch

  6. See the three merge methods with your own eyes

    You can reproduce what each button does with plain Git in a scratch repository. Start with main at three commits and feature/festival-offer holding three commits made from the second one, then run each method in its own clone.

    Terminal
    git log --oneline --graph --all
    Output
    * 23a53f9 docs: explain how to run tests
| * 211e2b9 test(discount): cover festival rate
| * f6d5bbb fix: typo
| * e3c8809 feat(discount): add festival offer rate
|/
* c3dfb8e docs: add README
* 3377390 feat: add discount module
    Terminal
    git merge --no-ff feature/festival-offer -m "Merge pull request #42 from your-org/feature/festival-offer"
git log --oneline --graph main
    Output
    Merge made by the 'ort' strategy.
 discount.js      | 1 +
 discount.test.js | 1 +
 2 files changed, 2 insertions(+)
 create mode 100644 discount.test.js
*   8155555 Merge pull request #42 from your-org/feature/festival-offer
|\
| * 211e2b9 test(discount): cover festival rate
| * f6d5bbb fix: typo
| * e3c8809 feat(discount): add festival offer rate
* | 23a53f9 docs: explain how to run tests
|/
* c3dfb8e docs: add README
* 3377390 feat: add discount module

    Compare the IDs. The merge commit keeps e3c8809, f6d5bbb and 211e2b9. Squash leaves none of them on main. Rebase keeps the three messages, but as 477f281, f67f1f6 and dac183d: same changes, new parents, new IDs. Your IDs will differ, because they include your name and the time.

Bring it to your team

Use these to agree how PRs work on your team, then write the answers into the PR template and CONTRIBUTING file (see Your team's engineering playbook).

  • "What is our size limit for a PR, and what do we do when a feature is bigger than that?"
  • "What is our target time for a first review, and who covers when the owner is on leave?"
  • "Which merge methods do we allow on main, and why? Can we turn on automatic branch deletion?"
  • "Which paths are risky enough to need a specific owner in CODEOWNERS: migrations, payments, pricing, workflows?"
  • "If someone pushes new commits after approval, does the approval still count? Should it?"
  • "What must a PR description contain before you will review it: issue link, test plan, screenshots?"

Red flags

  • PRs with thousands of changed lines approved within minutes.
  • Authors merging their own PRs without any approval, or admins routinely bypassing the rules.
  • Empty descriptions, or descriptions that only say "fixes".
  • Workflow, migration or payment files changed with no owner asked to review.
  • Review comments that attack the person, or a review queue nobody looks at for days.

Practice

Which issues will this PR close?

Mangrove's bot posts "this PR will close ..." on every new PR. Write issuesClosedOnMerge(pr, defaultBranch). pr is { base, body }. Return the issue references the PR will close when it merges, in order of first appearance, without duplicates. GitHub's rules: a keyword (close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved) in any case, as a whole word, optionally followed by a colon, then a space and a reference: #123 or owner/repo#123. Each reference needs its own keyword. Nothing closes unless the PR targets the default branch.

JavaScript exerciseRuns in your browser with tests
function issuesClosedOnMerge(pr, defaultBranch) {
  // pr.base: the branch the PR merges into
  // pr.body: the PR description
  // return e.g. ['#123', 'your-org/store-app#88']
  return [];
}
Ctrl + Enter runs
Show a solution
JavaScript
function issuesClosedOnMerge(pr, defaultBranch) {
  if (pr.base !== defaultBranch) return []; // keywords act only on the default branch
  const re = /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?):?\s+((?:[\w.-]+\/[\w.-]+)?#\d+)/gi;
  const refs = [];
  for (const m of pr.body.matchAll(re)) {
    if (!refs.includes(m[1])) refs.push(m[1]);
  }
  return refs;
}
Pass the exercise to finish this task.

Read this PR as a PM in ten minutes

You own the Store app's checkout receipt. This PR is waiting for review. Using the six-step read from this lesson, write three review comments and choose a verdict (Comment, Approve or Request changes).

Output
#231  feat(receipt): show delivery slot on receipt
Description: "Adds the slot. Fixes #214"
Checks: lint passed, test (22) passed, test (24) passed, e2e skipped
Files changed (+412 -37):
  src/receipt/receipt.ts
  src/receipt/receipt.test.ts
  prisma/schema.prisma
  prisma/migrations/20261014_add_slot/migration.sql
  .github/workflows/ci.yml
  package.json
  package-lock.json
Screenshots: none
  • You ask for a screenshot, because the receipt is something customers and store staff see
  • You ask about the migration: is it safe during trading hours, and how would it be rolled back?
  • You ask why ci.yml changed and why the e2e check was skipped in a PR about a receipt
  • You ask what the package.json change adds, or ask for it to be split out
  • You compare the PR with the acceptance criteria in issue #214, or ask for a test plan in the description
  • Your verdict is Comment (or Request changes for a clear blocker), not Approve, because you have not read the code
Show a model answer

1. "question: Could you add a screenshot of the receipt with a Now slot and a Later slot? Then we can check it against what issue #214 asked for."

2. "question: Does 20261014_add_slot lock the order table? Is it safe to run during trading hours, and what is the rollback?"

3. "question: Why does ci.yml change here, and is that why e2e was skipped? If the dependency in package.json is unrelated to the receipt, can it go in its own PR?"

Verdict: Comment. The answers decide whether a code owner should request changes; your role is to make sure the questions get asked and answered on the PR.

Check yourself

Your PR has six commits, including "wip" and "fix typo". The team wants each PR to appear on main as a single commit that can be reverted in one step. Which merge method fits?

CODEOWNERS has * @your-org/backend on line 1 and /prisma/ @your-org/data on line 5. A ready PR changes only prisma/schema.prisma. Who is asked to review?

You approved a PR yesterday. Today the author pushed two commits that change the payment logic. The branch rules require one approval and nothing else. What is true?

Flashcards

Open my review

Key takeaways

  • A PR points at a branch. Every push updates it and re-runs the checks; the diff is measured from the merge base.
  • Small PRs with a clear "why", a test plan, screenshots and a linked issue get faster and better reviews.
  • Reviewers comment, suggest exact changes, and finish with Comment, Approve or Request changes. Resolve threads when they are done.
  • CODEOWNERS routes reviews by path; the last matching line wins and owners need write access.
  • Merge commit keeps everything, squash makes one commit per PR, rebase copies commits with new IDs. Pick one default as a team, and read PRs as a PM from the description and the file list.

Sources

Episode transcriptThe second pair of eyes on every change · about 13 minutes · tap any line to play from there
  1. NadiaImran, picture the Thursday before a festival rush. A developer changes the discount code for partner shops, pushes it straight to main, and goes home. Friday morning, every partner shop is giving ten times the discount.
  2. ImranThat is my worst festival-week nightmare. And it is not far from things I have seen happen with price files.
  3. NadiaToday is about the habit that stops it. The pull request, and the code review that happens inside it.
  4. ImranI have opened plenty of pull requests in my developer years. But I want to understand them well enough to set the rules for my team.
  5. NadiaThen start with a price list. Before a festival rush, the pricing team prepares a new list for three hundred and twenty stores. One person types it. A second person checks every line against the approved offer sheet before it goes out.
  6. ImranAnd if she finds milk at fifteen cents instead of a dollar fifty, she circles it and sends it back.
  7. NadiaExactly. Nobody feels insulted. It is how a chain avoids selling milk at a tenth of the price for a weekend. A pull request is that second check for code.
  8. ImranSo the branch is the draft list, the reviewer is the second person, and merge is the moment the list goes to the stores.
  9. NadiaAnd the automated checks are the calculator that adds up every column. Lint, tests, build. They run on every push.
  10. ImranLet me check one thing I always half knew. Is a pull request a copy of my code?
  11. NadiaNo, and that matters. It points at your branch. Push one more commit to that branch, and it appears in the pull request at once, and the checks run again.
  12. ImranSo there is no "update the pull request" step. You just push.
  13. NadiaRight. The page has four tabs. Conversation tells you why the change exists. Commits shows the history. Checks shows the automated results. And Files changed is the diff, where the review happens.
  14. NadiaIf you can see your screen, the first diagram shows the life of a pull request. If not, picture a ladder going down: open, checks run, code review, merge, and at the bottom the linked issue closes.
  15. ImranAnd there is a side loop, I guess, for when the reviewer is not happy.
  16. NadiaYes. Request changes sends it to the author. The author pushes a fix, the checks run again, and it comes back for review. That loop can go round a few times.
  17. ImranSo how do I write a pull request that comes back with a yes on the first round?
  18. NadiaThree habits. First, keep it small. Eighty changed lines get read carefully. Fifteen hundred lines get skimmed and approved with "looks good to me".
  19. ImranIs there a number?
  20. NadiaMany teams aim for under roughly four hundred changed lines, not counting lockfiles and generated code. For bigger work, split it into a chain. First the database change, then the API, then the screen.
  21. ImranSecond habit?
  22. NadiaWrite the description for a stranger. What changed, why, how you tested it, where the reviewer should start, and what could go wrong. Link the issue with a closing keyword, like the word Fixes followed by the issue number, so it closes on merge.
  23. ImranAnd screenshots. If a reviewer can see the new receipt on the Partner app, she does not have to imagine it.
  24. NadiaYes. Third, use a draft while the work is unfinished. A draft cannot be merged, and code owners are not pinged yet. It lets CI run and lets you ask one early question without asking for a full review.
  25. ImranNow switch sides. I am the reviewer. What tools do I have?
  26. NadiaYou can comment on one line or drag across a block of lines. And there is a suggestion button. It drops the selected lines into a special block, you edit them into what you want, and the author can apply it with one click, called Commit suggestion.
  27. ImranThat is much nicer than writing "please rename this variable" and waiting a day.
  28. NadiaIt is. Another good habit is to click Start a review instead of posting each comment at once. Your comments stay private until you submit, so you can delete the ones that the next file answers.
  29. ImranAnd at the end I choose a verdict.
  30. NadiaOne of three. Comment means feedback, no decision. Approve means you are happy for it to merge. Request changes means something must be fixed first, and when the branch rules require reviews, it blocks the merge until that reviewer approves or someone with write access dismisses the review.
  31. ImranHere is a trap I want to test. I approved a pull request yesterday. Today the author pushed two more commits. My approval is gone, right? GitHub must remove it.
  32. NadiaThat is a very reasonable guess, Imran, and it is wrong. By default your approval stays, even though you never saw the new code.
  33. ImranSo someone could get an approval and then slip something in.
  34. NadiaThey could. That is why rulesets have options to dismiss stale approvals when new commits arrive, or to require approval of the most recent push. We set those in the next lesson.
  35. ImranGood. What about getting the right people to review? On my team, payment code should never be reviewed only by the person who sits nearest.
  36. NadiaThat is the CODEOWNERS file. It maps paths to owners. The payments folder belongs to the payments team, migrations belong to the backend leads. When a pull request touches those paths, GitHub asks those owners automatically.
  37. ImranAnd if two lines match the same file?
  38. NadiaThe last matching line wins. So you put the general rule at the top, everything to the backend team, and the specific rules below it. Owners also need write access to the repository, or the line is ignored.
  39. ImranDo all the owners have to approve?
  40. NadiaNo. With the code owner rule turned on, one approval from any listed owner for each changed path is enough. Think of it as a routing table, not a committee.
  41. ImranNow the merge button. It has three modes and I have never been fully sure what each one does to history.
  42. NadiaThe second diagram shows it. Imagine a pull request with three commits, A, B and C. Merge commit keeps all three and adds one more commit with two parents that ties them in.
  43. NadiaSquash and merge throws away A, B and C as separate commits and puts one new commit on main that contains all three changes. Rebase and merge copies A, B and C onto the tip of main, one by one, and each copy gets a new ID.
  44. ImranWhy a new ID if the change is the same?
  45. NadiaBecause a commit ID is a fingerprint of the commit, including its parent. New parent, new fingerprint. You saw that in the lesson on the inside of the dot git folder.
  46. ImranSo which one should my team use?
  47. NadiaFor most product teams, squash is the simplest default. One pull request becomes one commit. Easy to read, easy to revert in one step, and the titles can feed a changelog.
  48. ImranAny trap with squash?
  49. NadiaOne big one. If you keep working on the same branch after it was squashed, your next pull request contains commits that are already on main in squashed form, and you get conflicts. So delete the branch after merging and start fresh from main.
  50. ImranNow my real question. I am a PM. How do I read a pull request without reading every line of TypeScript?
  51. NadiaWork down the page. Title and linked issue first: does it match what was asked? Then the description: is there a why, a test plan and a risk note? Then screenshots, for copy, translated text and error states.
  52. NadiaThen the checks. And then the file list, without the code. You are looking for risky files: database migrations, the workflow folder, package files and the lockfile, config, anything about payments or pricing.
  53. ImranAnd whether test files changed too. If the discount logic changed and no test did, that is my question.
  54. NadiaExactly. And size. Thirty files for a copy change means something else is riding along. Ask in review comments, so the answers stay with the code.
  55. ImranShould I approve if it all looks fine to me?
  56. NadiaUnless your team agrees otherwise, leave a comment instead. An approval counts toward the merge rule, so it should come from someone who has read the code.
  57. ImranI did exactly that this week. The festival discount pull request on the partner API had four files: the discount service, its test, a database migration and the CI workflow file.
  58. NadiaAnd which two did you ask about?
  59. ImranThe migration, because eighteen hundred partner shops place orders all day, and the workflow file, because I could not see why a discount change touches CI. I left both as comments and did not approve.
  60. NadiaThat is the whole skill. You never judged the TypeScript. You checked that it matched the issue, that it was tested, and that the owners of the risky files had looked.
  61. NadiaIn the Build it section you do all of this for real. You add a pull request template, so every description starts with sections for what and why, testing, screenshots and risk.
  62. ImranThen a CODEOWNERS file with the general rule first and the risky folders after it.
  63. NadiaThen you open a draft pull request from the terminal with the GitHub command line tool, watch the checks, mark it ready, review someone else's with request changes and approve, and merge with squash while deleting the branch.
  64. ImranAnd the last step is my favourite. You reproduce the three merge buttons with plain Git in a scratch repo, and compare the commit IDs.
  65. NadiaThat is where it clicks. The rebase result has the same three messages with three brand new IDs.
  66. NadiaThe practice section has two tasks. One is a small coding exercise in your browser: given a pull request description, work out which issues it will close. Keywords like fixes or closes, one keyword per issue, and only when it merges into the default branch.
  67. ImranAnd the other is a pull request to read as a PM, with a migration, a CI change and no screenshot. I write three questions and pick a verdict.
  68. ImranFor my next team meeting, I have three questions. What is our size limit for a pull request, and what do we do when a feature is bigger?
  69. ImranWhat is our target time for a first review, and who covers when the owner is on leave? And which risky paths need a named owner in CODEOWNERS?
  70. NadiaI would add one. If someone pushes after approval, should the approval still count? Decide it on purpose.
  71. NadiaHere is one to think about. CODEOWNERS gives everything to the backend team on line one, and the prisma folder to the data team on line five. A pull request changes only the schema file in that folder. Who gets asked? Take a second.
  72. ImranI want to say both teams, but you told me the last match wins.
  73. NadiaRight. Only the data team. The later, more specific line overrides the general one for that file.
  74. ImranLet me sum up. One. A pull request points at a branch, so every push updates it and runs the checks again.
  75. ImranTwo. Small pull requests with a why, a test plan and screenshots get better reviews, and reviewers finish with comment, approve or request changes.
  76. ImranThree. Squash gives one commit per pull request, which is the easiest history to read and revert.
  77. NadiaOne nuance on the first point. An approval does not reset when new commits arrive, unless you turn that rule on. Which is exactly where the next episode starts: branch rules, releases and version numbers.
Finished reading?Mark it complete to fill in its stop on your course line.

Use it at work

Unit-test starter

A function with a test file: the happy path, the edges and the errors, grouped and named clearly.

Open in the Code Lab
Next stopBranching strategies, releases and versions
Was this lesson helpful?
Questions and discussion

Loading the discussion…