GitHub pull requests and code review
A pull request asks for a branch to be merged into main and gives the team a place to review it first. Reviewers comment on the exact lines, automated checks run, and once the required approvals and checks pass, the branch is merged and usually deleted.
From branch to main, with review
- Push your branch to GitHub.
- Open a pull request (PR) from your branch into
main. Give it a clear title and say what changed, why, and how you tested it. - Reviewers read the diff and comment on specific lines, ask questions, request changes or approve.
- Automated checks run at the same time, such as the build and the tests, often from Jenkins or GitHub Actions. Each shows a green tick or a red cross on the PR.
- You push fixes to the same branch. The PR updates itself and the checks run again.
- When the rules are met, someone merges it, and the branch is usually deleted.
Rules that protect main
Repository administrators set branch protection rules on main (GitHub's newer *rulesets* do the same job). Typical rules: no direct pushes, at least one approval, all required checks green, and the branch up to date with main. These rules make 'someone reviewed it and the tests passed' a guarantee rather than a hope.
Three ways to merge a PR
| Option | What ends up on main |
|---|---|
| Create a merge commit | All of the branch's commits, plus a merge commit joining them |
| Squash and merge | One single commit containing the whole change. Keeps main's history short |
| Rebase and merge | The branch's commits replayed on top of main, with no merge commit |
Teams pick one style and stick to it. Many choose squash, so that each PR becomes one tidy commit on main.
Forks
On a team repository you normally push branches to the same repository. When you have no write access, as with open-source projects, you fork it: GitHub makes your own copy, you push your branch there, and you open the pull request from your fork back to the original.
Reviewing well
- Keep pull requests small. A reviewer can check 200 lines properly, but not 5,000.
- Comment on the code, not the person: 'This loop reads the file twice. Could we read it once?'
- Check what tests cannot: is the change what the ticket asked for, is it readable, and what happens on bad input?
What is the name of the GitHub feature that asks for a branch to be reviewed and merged into main? (two words)
Show a hint
You ask the project to 'pull' your branch.
Show the solution
A pull request (PR).
Common mistakes
Large PRs get skimmed, not reviewed. Split the work into small PRs that each make sense on their own.
A failing check is the pipeline telling you something is broken. Fix it, or find out why, before you merge.
Reviewers need to know what changed, why, and how you tested it. Two or three sentences save a lot of questions.
What you will see at work
- Most teams require at least one approval before anything reaches main. Reviewing other people's PRs is part of the job.
- The green tick on a PR usually comes from the CI pipeline you will meet in the Jenkins course.
- Auditors like pull requests: each change to production code has a reviewer, a date and a record of the checks.
Key terms
Check your understanding.
Take this lesson's quiz and save your progress. Free.