Reviewing and merging pull requests

Alex's pull request #3, Add the network printer, is open, and Sam is listed as reviewer. Before it reaches main, Sam reads the changes, asks for a correction, approves the result, and merges it. Then both clean up their branches.

Requesting a review

Reviewers are chosen in the Reviewers section of the side panel, when the pull request is created or at any time later: click the gear icon, then search for a person (or, in an organization, a team). Up to 15 reviewers can be requested. Each one is notified, and the pull request appears in their review requests.

A reviewer must have access to the repository, and cannot be the author: pull request authors cannot approve their own pull requests.

Repositories can also assign reviewers automatically, with a CODEOWNERS file that names the owners of each part of the code.

Reviewing the changes

The review happens in the Files changed tab. Each file is shown as a diff: deleted lines in red, prefixed with -; added lines in green, prefixed with +. Two layouts are available: Unified (one column) and Split (before and after side by side).

Commenting on lines

1. Hover over a line, and click the blue + icon that appears. To comment on several lines, click and drag over the line numbers.
2. Write the comment, in Markdown.

3. Choose:

   Button	Effect
   Add single comment	Posts the comment immediately, alone
   Start a review	Keeps the comment pending, invisible to others, until the review is submitted. The next comments use Add review comment

Starting a review is the usual choice: the author receives all the comments at once, with a summary, instead of one notification per comment.

Sam comments on the line printer01,192.168.1.30,printer:

Can you also add the printer to services.md, with its print server?

Suggesting a change

In a line comment, the suggestion icon (a file with + and -) inserts a suggestion block containing the line. Edit it into what the line should be:

```suggestion
printer01,192.168.1.30,printer
```

The author can then accept it with Commit suggestion, which creates the commit directly on the branch. Several suggestions can be batched into a single commit.

Marking files as viewed

The Viewed checkbox in the header of each file collapses it, and counts the reviewed files (1 / 2 files viewed). If the file changes afterwards, it is unchecked again.

Submitting the review

Click Review changes (top right of Files changed), write a summary, choose one of the three types of review, and click Submit review:

Type	Meaning	Effect on the merge
Comment	General feedback, suggestions, questions, with no requirement to change the project	None
Approve	The changes are good to merge	Counts towards the required approvals
Request changes	Feedback that must be addressed before merging	Blocks the merge on protected branches until the reviewer approves or the review is dismissed

Sam submits a Request changes review. The Conversation tab now shows Changes requested, and the merge box says that one review requests changes.

Answering a review

The author, Alex, answers in the review's threads, then fixes the branch:

$ echo "- printer01: CUPS print server" >> services.md
$ git commit -am "Document printer in services"
$ git push

The new commit appears in the pull request. Then:

• Resolve conversation: each thread can be marked as resolved once addressed. It is collapsed, but stays readable. (Branch protection can require all conversations to be resolved before merging.)
• Re-request review: the circular arrows icon next to the reviewer's name, in the side panel, notifies them that the pull request is ready for another look.

Sam reviews again, and submits an Approve review. The merge box shows Changes approved.

Users with write access can also dismiss a review (with a message), for example when the reviewer is away and the change has been made.

Merging a pull request

The merge box, at the bottom of the Conversation tab, sums up the state of the pull request:

Line	Meaning
Changes approved / Changes requested / Review required	The status of the reviews
All checks have passed	Automated checks, if any
This branch has no conflicts with the base branch	Can be merged automatically
This branch has conflicts that must be resolved	Use Resolve conflicts (web editor, for simple conflicts) or resolve locally, then push

The Merge pull request button has an arrow to choose the merge method:

Method	Result on main	Equivalent
Create a merge commit	All the commits of the branch, plus a merge commit Merge pull request #3 from alex-martin/add-printer	git merge --no-ff
Squash and merge	A single new commit containing all the changes, titled after the pull request: Add the network printer (#3)	git merge --squash then git commit
Rebase and merge	The commits of the branch, replayed one by one on top of main, without a merge commit	git rebase then a fast-forward

%%{init: {"gitGraph": {"showBranches": true, "rotateCommitLabel": false}, "themeVariables": {"git0": "#43a047", "git1": "#1e88e5", "gitBranchLabel0": "#ffffff", "gitBranchLabel1": "#ffffff", "gitInv0": "#ffffff", "commitLabelFontSize": "13px"}}}%%
gitGraph TB:
    commit id: "Create inventory"
    branch add-printer
    commit id: "Add network printer"
    commit id: "Document printer in services"
    checkout main
    merge add-printer id: "Merge pull request #3"

After choosing, edit the commit message if needed, and click Confirm merge (or Confirm squash and merge, Confirm rebase and merge). The pull request becomes Merged (purple), and closes the issues it references with a closing keyword.

The methods offered are chosen in Settings > General > Pull Requests. The same section has Automatically delete head branches, which deletes the branch of each pull request once it is merged.

Deleting and restoring the branch

Once merged, the branch has no more use. The merge box shows Pull request successfully merged and closed, with a Delete branch button. The timeline then records deleted the add-printer branch, with a Restore branch button, in case it is needed again (for example, to continue the work).

Closed and merged pull requests no longer appear in the default list of the Pull requests tab. Click Closed next to Open, or filter:

Filter	Shows
is:pr is:open	Open pull requests (the default)
is:pr is:closed	Closed pull requests, merged or not
is:pr is:merged	Merged pull requests
is:pr review-requested:@me	Pull requests waiting for your review

From a closed pull request, Restore branch recreates its branch at its last commit, even days later. A merged pull request also offers Revert: GitHub creates a branch and a new pull request that undoes its changes, like git revert.

After the merge: cleaning up locally

The merge happened on GitHub. Local clones still have the old main and the feature branch. With a merge commit, everything lines up after a pull:

$ git switch main
Switched to branch 'main'
Your branch is up to date with 'origin/main'.
$ git pull --prune
From https://github.com/sam-rivera/homelab
 - [deleted]         (none)     -> origin/dns
   a8c9f40..f34a205  main       -> origin/main
Updating a8c9f40..f34a205
Fast-forward
 README.md | 1 +
 1 file changed, 1 insertion(+)
$ git branch -d dns
Deleted branch dns (was 5341104).

• --prune removes the remote-tracking branches deleted on GitHub (origin/dns).
• git branch -d accepts, because the branch's commits are in main.

With squash and merge, the commits of the branch never reach main: only a new commit with the same changes does. Git cannot know they are equivalent, so the safe deletion refuses:

$ git pull --prune
From https://github.com/sam-rivera/homelab
 - [deleted]         (none)     -> origin/docs
   6ec82f1..a8c9f40  main       -> origin/main
Updating 6ec82f1..a8c9f40
Fast-forward
 README.md | 2 ++
 1 file changed, 2 insertions(+)
$ git branch -vv
  docs 90cafe6 [origin/docs: gone] Document gateway
* main a8c9f40 [origin/main] Document the network (#1)
$ git branch -d docs
error: the branch 'docs' is not fully merged
hint: If you are sure you want to delete it, run 'git branch -D docs'
hint: Disable this message with "git config set advice.forceDeleteBranch false"
$ git branch -D docs
Deleted branch docs (was 90cafe6).

[origin/docs: gone] confirms that the branch was deleted on GitHub. Once you have checked that the pull request was merged, -D is safe.

Summary

Action	Where
Request a review	Reviewers in the side panel
Comment on a line	Files changed > + on the line > Start a review
Propose an exact change	Suggestion block, accepted with Commit suggestion
Submit the review	Review changes > Comment / Approve / Request changes > Submit review
Answer a review	Reply in the threads, push fixes, Resolve conversation, Re-request review
Merge	Merge pull request (merge commit, squash, or rebase) > Confirm merge
Delete or restore the branch	Delete branch / Restore branch on the pull request
Find closed pull requests	Closed, or is:pr is:closed
Update the local clone	git switch main, git pull --prune, git branch -d <branch>

Common mistakes

• Posting each line comment separately. Start a review, so the author receives one consistent review.
• Approving a pull request you have not read. An approval is a statement that the change is correct.
• Requesting changes for a matter of taste. Use a comment for suggestions that are not required.
• Forgetting to re-request a review after fixing. The reviewer may not notice the new commits.
• Using git branch -d after a squash merge and concluding the work was lost. The changes are in main, as a single commit: delete the branch with -D.

Hands-on labs

Three labs, from guided to more autonomous. They continue the repository homelab-practice-10 and its pull requests from Pull requests; Lab 2 also needs Git and a token with write access. Lab 3 is best with a second account. Replace <username> with your GitHub username.

Lab 1: review, merge, clean up

Objective: review your own pull request with comments and a suggestion, merge it with a merge commit, and clean up on GitHub and locally.

Prerequisites and initial state: homelab-practice-10, with the open pull request #2 (Add the network printer, branch add-printer) from Lab 1 of the previous page, which closes issue #1.

Setup:

mkdir -p ~/git-practice/review && cd ~/git-practice/review
git clone https://github.com/<username>/homelab-practice-10.git
cd homelab-practice-10
git switch add-printer
git switch main

Tasks:

1. In Files changed of #2, add a comment on the printer01 line with Start a review, containing a suggestion that changes printer to print-server. Mark both files as Viewed.
2. Open Review changes: which review types are available to you? Submit a Comment review.
3. In the Conversation tab, commit the suggestion, then resolve the conversation.
4. Merge the pull request with Create a merge commit, and delete the branch. Then restore it, and delete it again.
5. Check issue #1.
6. In the clone, update main, removing the deleted remote branch, and delete the local add-printer safely. Display the graph of the history.

Expected result and verification:

• Task 2: Approve and Request changes are disabled: you are the author.
• Task 3: a commit Update inventory.csv appears in the pull request; the conversation is collapsed as resolved.
• Task 5: issue #1 is Closed as completed, closed by pull request #2.
• Task 6: git pull --prune prints - [deleted] (none) -> origin/add-printer; git branch -d add-printer succeeds; git log --oneline --graph shows Merge pull request #2 from <username>/add-printer joining the branch's three commits.

Solution

1. On the line: + > suggestion icon > edit to printer01,192.168.1.30,print-server > Start a review. Tick Viewed on inventory.csv and services.md.
2. Review changes > Comment > Submit review.
3. Commit suggestion > Commit changes; then Resolve conversation.
4. Merge pull request > Confirm merge > Delete branch > Restore branch > delete again on the branches page.

cd ~/git-practice/review/homelab-practice-10
git switch main
git pull --prune                   # - [deleted] (none) -> origin/add-printer
git branch -d add-printer          # Deleted branch add-printer
git log --oneline --graph

• The local add-printer did not have the suggestion commit; -d still succeeds, because all its commits are in main.
• Restoring a branch recreates it at the same commit: nothing is lost by deleting a merged branch.

Keep the clone and the repository for Lab 2.

Lab 2: conflict, squash, and the stubborn branch

Objective: resolve a conflict in a pull request, merge it with Squash and merge, and clean up a squashed branch locally.

Prerequisites and initial state: the clone from Lab 1, and the pull request Fix pi-dns IP address (branch fix-dns-ip) from Lab 2 of the previous page, which conflicts with main. A token with write access to the repository.

Tasks:

1. In the clone, update the local fix-dns-ip from the remote, merge origin/main into it, and resolve the conflict by keeping 192.168.1.11. Commit and push.
2. On GitHub, check the merge box of the pull request, then merge it with Squash and merge. Delete the branch.
3. Look at the history of main on GitHub (N commits).
4. In the clone, switch to main, update it, then delete fix-dns-ip with the safe option. Read the message, check git branch -vv, and finish the deletion.

Expected result and verification:

• Task 1: the merge stops with CONFLICT (content): Merge conflict in inventory.csv; after the push, the merge box shows This branch has no conflicts with the base branch.
• Task 3: a single new commit on main, titled Fix pi-dns IP address (#3); the branch's commits and its merge commit do not appear.
• Task 4: git branch -d fix-dns-ip fails with error: the branch 'fix-dns-ip' is not fully merged; git branch -vv shows [origin/fix-dns-ip: gone]; git branch -D fix-dns-ip deletes it.

Solution

cd ~/git-practice/review/homelab-practice-10

# 1. Resolve the conflict on the branch
git fetch
git switch fix-dns-ip
git pull                            # up to date with origin/fix-dns-ip
git merge origin/main               # CONFLICT (content): Merge conflict in inventory.csv
sed -i -e '/^<<<<<<< /d' -e '/^=======/,/^>>>>>>> /d' inventory.csv   # keep our side (.11)
cat inventory.csv
git add inventory.csv
git commit --no-edit
git push

# 4. Clean up after the squash merge
git switch main
git pull --prune                    # - [deleted] (none) -> origin/fix-dns-ip
git branch -d fix-dns-ip            # error: the branch 'fix-dns-ip' is not fully merged
git branch -vv                      # fix-dns-ip ... [origin/fix-dns-ip: gone]
git branch -D fix-dns-ip

• The sed command keeps the lines between <<<<<<< and ======= (the branch's side). It assumes Git's default conflict style; with merge.conflictStyle set to diff3 or zdiff3, edit the file by hand. Editing the file by hand works just as well; check the result with cat before committing.
• Resolve conflicts on GitHub would also work for this simple conflict: it commits the resolution to the branch from the browser.
• Squashing gives main a clean history, at the cost of losing the individual commits of the branch on main.

Clean up when you are done: rm -rf ~/git-practice/review, and delete homelab-practice-10 on GitHub (Settings > General > Danger Zone), along with the token used for the lab.

Lab 3: final lab, from issue to merge

Objective: combine everything from this part: repository, protection, access, issue, branch, pull request, review, and merge, without step-by-step help.

Prerequisites and initial state: a GitHub account, Git, and a token. Ideally a second account playing Alex (otherwise, see the solo variant below). No setup: you start from nothing.

Tasks:

1. (Sam) Create a public repository homelab-final with a README and the Python .gitignore. Protect main so that changes require a pull request with one approval (administrators included). Invite Alex as a collaborator.
2. (Sam) Open an issue Add the backup script, assigned to Alex, with a task list of two items.
3. (Alex) Clone the repository, and try to commit and push directly to main (then undo that commit with git reset --hard origin/main). Create a branch, add scripts/backup.py, push, and open a pull request that closes the issue, with Sam as reviewer.
4. (Sam) Request changes: ask for a docstring at the top of the script.
5. (Alex) Push a fix, and re-request the review.
6. (Sam) Approve, and merge with Squash and merge. Delete the branch.
7. (Alex) Update the local clone, and delete the local branch.

Solo variant (one account): do the steps of both roles yourself, but untick Do not allow bypassing the above settings in the protection rule. At step 4, submit a Comment review instead of Request changes; at step 6, merge using the administrator bypass (GitHub offers to merge without waiting for the requirements). Note every place where GitHub stops you because you are the author.

Expected result and verification:

• Pushing directly to main from the clone is refused (protected branch hook declined).
• The issue is closed as completed by the pull request, and the Development sections link them.
• The pull request's timeline shows, in order: review requested, changes requested, new commit, review re-requested, approved, merged, branch deleted.
• main ends with Initial commit and one squashed commit; both clones are on main, up to date, with no leftover branch.

Solution

1. New repository (README, Python .gitignore); Settings > Branches > classic rule on main with Require a pull request before merging, Require approvals: 1, Do not allow bypassing the above settings; Settings > Collaborators > Add people.
2. Issues > New issue, Assignees: Alex.

3. As Alex:

   git clone https://github.com/<sam>/homelab-final.git
   cd homelab-final
   git config user.name "Alex Martin"
   git config user.email "[email protected]"
   echo "test" >> README.md && git commit -am "Direct change"
   git push origin main               # ! [remote rejected] main -> main (protected branch hook declined)
   git reset --hard origin/main       # drop the refused commit
   git switch -c add-backup-script
   mkdir scripts && printf 'print("backup")\n' > scripts/backup.py
   git add scripts/backup.py
   git commit -m "Add NAS backup script"
   git push -u origin add-backup-script
   

   Then Compare & pull request, description Closes #1, Reviewers: Sam.

4. As Sam: Files changed > comment on line 1 > Start a review > Review changes > Request changes > Submit review.

5. As Alex:

   printf '"""Back up the NAS shares."""\nprint("backup")\n' > scripts/backup.py
   git commit -am "Add docstring to backup script"
   git push
   

   Then the Re-request review icon next to Sam.

6. As Sam: Review changes > Approve; Squash and merge > Confirm squash and merge > Delete branch.

7. As Alex:

   git switch main
   git pull --prune
   git branch -D add-backup-script    # -d refuses after a squash
   

8. Every step of this lab maps to one page of this part: you have used the whole GitHub flow.

Clean up when you are done: delete homelab-final on GitHub, the tokens you created, and the local clones.