Read and review diffs

Lesson 8
40 min
Module 3: Collaborate through reviews

Lesson 8 of 15 · Git and Team Development EssentialsRead and review diffs

Course overview

1. Understand a repository

2. Work with branches and remotes

3. Collaborate through reviews

4. Resolve and recover

5. Release and hotfix practice

What you will be able to do

Find the problems in a pull request before they reach main. After this lesson, you can review a branch with a checklist, and write review comments that the author can act on.

Before you startWhat to know and to have readyAbout 40 minutes

What you need to know

The steps of a pull request, and the source and target branches, from Pull-request sequence: What a pull request is.
How to read the - and + lines of git diff, from Track a first change: Worked example: see a change before you save it.

What you need on your computer

The folder ~/projects/nextia-learning-labs. You cloned it in Pull-request sequence: Set up the practice lab. If it is missing, do step 2 of that section.
The practice lab in the state m03. Reset it, so that your output is the same as this lesson. In ~/projects, run:
Terminal window
```
sh nextia-learning-labs/C14/setup.sh m03 --reset
```
The reset deletes the whole c14-lab folder, with your PR files from the last lesson. Copy them to another folder first if you want to keep them.
Then go to ~/projects/c14-lab/ticket-notes. git status shows On branch main and working tree clean. Sam’s branch add-ticket-104, which you review in this lesson, is on the remote. The script made it.

What a reviewer looks for

Sam opened a pull request: add-ticket-104 → main. His PR description is short:

# Add ticket 104

Source: add-ticket-104. Target: main.

## Problem
Ticket 104 is not in the notes.

## Change
Add ticket 104 to tickets.txt.

A review is more than “the change looks fine”. You compare what the PR says with what the branch really contains. Use this checklist for every PR:

Check	Question	How to answer it
Scope	Which commits and files does the PR contain?	`git log --oneline main..<branch>` and `git diff --stat main...<branch>`
Behaviour	Does each change do what the description says?	Read the full diff, and `git show` each commit.
Checks and tests	Does the check pass on the branch?	Switch to the branch, and run `sh check.sh`.
Accidental files	Is there a file that does not belong: notes, logs, secrets, generated files?	Look for unexpected names in `git diff --stat`.
Unrelated changes	Is there a change that the description does not mention?	Compare each commit with the Change section.
Messages	Does each commit message describe its change?	`git show <hash>`. The message is not the change.

Two dots or three dots

Before the worked example, you need one detail of the commands. git log and git diff both accept A..B and A...B, but the dots mean different things:

Command	Shows
`git log main..add-ticket-104`	The commits on `add-ticket-104` that are not on `main`. These are the commits of the PR.
`git diff main...add-ticket-104`	The changes that `add-ticket-104` made since it started from `main`. This is what the PR adds.
`git diff main..add-ticket-104`	The difference between the two newest commits. It also shows, in reverse, every change that `main` got after the branch started.

So, for a review, use two dots with git log and three dots with git diff. The common mistake at the end of this lesson shows what goes wrong with two dots in git diff.

The commit where a branch started is its merge base. Three dots compare the branch with its merge base, not with the newest main.

Worked example: Amira reviews add-ticket-104

Current branch: main in Amira’s clone, ticket-notes. PR: add-ticket-104 → main.

1. Get the newest version of the branch

git fetch

No output means that her record of the remote is already up to date. Always fetch before you review. Without a fetch, you review an old version of the branch.

2. Scope: commits and files

git log --oneline main..origin/add-ticket-104

5a15d07 Improve README wording
e34484f Add ticket 104

The PR is about ticket 104, but it has two commits. Improve README wording is not in the description. Amira notes it.

git diff --stat main...origin/add-ticket-104

 README.md   | 2 +-
 scratch.txt | 2 ++
 tickets.txt | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

--stat shows a summary: each file, and how many lines were added (+) and removed (-). The PR changes three files. The description mentions only tickets.txt. scratch.txt is a new file with a name that suggests personal notes.

3. Behaviour: the full diff

git diff main...origin/add-ticket-104

diff --git a/README.md b/README.md
index a30ec45..378c442 100644
--- a/README.md
+++ b/README.md
@@ -1,3 +1,3 @@
 # Ticket notes

-Notes about support tickets for the help desk.
+Notes about the support tickets that the help desk receives.
diff --git a/scratch.txt b/scratch.txt
new file mode 100644
index 0000000..8089905
--- /dev/null
+++ b/scratch.txt
@@ -0,0 +1,2 @@
+try restarting the VPN client?
+ask Amira about logs
diff --git a/tickets.txt b/tickets.txt
index 68cc955..0e68cf1 100644
--- a/tickets.txt
+++ b/tickets.txt
@@ -1,3 +1,4 @@
 Ticket 101: printer offline. Restarted the spooler.
 Ticket 102: password reset email not received.
 Ticket 103: shared drive is slow. Moved the user to the new server.
+ticket 104 - VPN drops every hour.

Amira finds three problems:

The new line in tickets.txt does not use the format of the other lines. It starts with ticket in lower case, and it has - instead of : .
scratch.txt contains Sam’s working notes. It does not belong in the repository.
The README change is a different topic from ticket 104.

4. One commit at a time

To see which commit made which change, she shows each commit:

git show e34484f

commit e34484fcb68e7a8521d428d1c42c28b37ba24720
Author: Sam Okafor <sam@example.com>
Date:   Tue Oct 6 09:08:00 2026 -0400

    Add ticket 104

diff --git a/scratch.txt b/scratch.txt
new file mode 100644
index 0000000..8089905
--- /dev/null
+++ b/scratch.txt
@@ -0,0 +1,2 @@
try restarting the VPN client?
ask Amira about logs
diff --git a/tickets.txt b/tickets.txt
index 68cc955..0e68cf1 100644
--- a/tickets.txt
+++ b/tickets.txt
@@ -1,3 +1,4 @@
 Ticket 101: printer offline. Restarted the spooler.
 Ticket 102: password reset email not received.
 Ticket 103: shared drive is slow. Moved the user to the new server.
ticket 104 - VPN drops every hour.

The message says Add ticket 104, but the commit also adds scratch.txt. This is why you read the change, and not only the message. git show 5a15d07 shows that the second commit changes only README.md.

5. Run the check on the branch

A reviewer runs the check on the files of the branch, not on main. Amira switches to the branch. Git makes a local add-ticket-104 that tracks origin/add-ticket-104, as you saw in Module 2:

git switch add-ticket-104
sh check.sh

Switched to a new branch 'add-ticket-104'
branch 'add-ticket-104' set up to track 'origin/add-ticket-104'.
FAIL: these lines in tickets.txt do not start with 'Ticket <number>: '
4:ticket 104 - VPN drops every hour.

The check fails on line 4. The PR description has no How I checked section, so Sam probably did not run the check. Amira switches back to main:

git switch main

Switched to branch 'main'
Your branch is up to date with 'origin/main'.

6. Write the review

A useful review comment is:

Specific. It names the file, the line or the commit, says what is wrong, and says what to do.
Marked as blocking or as a suggestion. A blocking comment must be fixed before the merge. A suggestion is an idea that the author can accept or leave.
Kind. It is about the change, not the person. “This line fails the check” is better than “You forgot the format again”.

The review ends with a verdict: approve, or request changes. One blocking comment is enough to request changes.

Simulation: Amira writes her review in ~/projects/c14-lab/review-add-ticket-104.md. On GitHub, she would write each comment on its line in the Files changed tab.

# Review of add-ticket-104 → main

Verdict: request changes.

1. Blocking. tickets.txt, line 4: "ticket 104 - VPN drops every hour."
   fails sh check.sh. Please use the format of the other lines:
   "Ticket 104: VPN drops every hour."
2. Blocking. scratch.txt looks like working notes, and commit e34484f
   added it. Please remove it from the branch.
3. Blocking. The README change in 5a15d07 is not about ticket 104.
   The new wording is clear, so please open a separate PR for it,
   and take it out of this one.
4. Suggestion. Add a How I checked section to the PR description,
   with the output of sh check.sh.

Guided practice: review the PR yourself

Simulation: you play Amira, the reviewer. Write down each thing that you find before you read the next step.

In ~/projects/c14-lab/ticket-notes, run git status. The output shows On branch main.
Get the newest version of the remote:
Terminal window
```
git fetch
```
List the commits of the PR:
Terminal window
```
git log --oneline main..origin/add-ticket-104
```
The output shows two commits: 5a15d07 and e34484f.
List the files that the PR changes:
Terminal window
```
git diff --stat main...origin/add-ticket-104
```
The output shows three files. Compare them with the PR description.
Read the full diff with git diff main...origin/add-ticket-104. Find the line that has a different format.
Show each commit:
Terminal window
```
git show e34484f
git show 5a15d07
```
Write down which commit added each file.
Switch to the branch, and run the check:
Terminal window
```
git switch add-ticket-104
sh check.sh
```
The output starts with FAIL, and shows line 4.
Run ls. scratch.txt is in the folder on this branch.
Switch back to main:
Terminal window
```
git switch main
```
Make ~/projects/c14-lab/review-add-ticket-104.md in your editor. Write your comments, marked as blocking or suggestion, and a verdict.
Compare your review with Amira’s review in the worked example. Did you find the format error, the accidental file and the unrelated change?

Your turn: fix the PR as Sam, then review it again

Change roles. You are Sam now, in sam-notes. Fix the three blocking problems on the same branch with new commits. Do not change or remove the old commits. Then push, and review the PR again as Amira.

In ~/projects/c14-lab/sam-notes, switch to add-ticket-104.
Fix line 4 of tickets.txt in your editor. Run sh check.sh, and commit.
Remove scratch.txt from the branch with git rm, and commit.
Put the README text back to the text in main, and commit. Use git show origin/main:README.md, which prints the file as it is on main.
Push.
As Amira, in ticket-notes: fetch, read the scope and the diff again, update your local add-ticket-104, and run the check. Decide the verdict.

Show a model answer

As Sam, in sam-notes. Switch to the branch. Git makes a local branch from origin/add-ticket-104:

cd ~/projects/c14-lab/sam-notes
git switch add-ticket-104

Change line 4 of tickets.txt to Ticket 104: VPN drops every hour. and save. Then check the change and commit:

git diff
sh check.sh
git commit -am "Fix the format of ticket 104"

diff --git a/tickets.txt b/tickets.txt
index 0e68cf1..4971bbf 100644
--- a/tickets.txt
+++ b/tickets.txt
@@ -1,4 +1,4 @@
 Ticket 101: printer offline. Restarted the spooler.
 Ticket 102: password reset email not received.
 Ticket 103: shared drive is slow. Moved the user to the new server.
-ticket 104 - VPN drops every hour.
+Ticket 104: VPN drops every hour.
PASS: 4 ticket lines checked

git rm deletes the file and stages the deletion in one step:

git rm scratch.txt
git commit -m "Remove scratch notes"

rm 'scratch.txt'

The next command writes the main version of README.md over the file in the folder. > replaces all the text in the file. Run it only for this file:

git show origin/main:README.md > README.md
git diff
git commit -am "Undo the README wording change"

diff --git a/README.md b/README.md
index 378c442..a30ec45 100644
--- a/README.md
+++ b/README.md
@@ -1,3 +1,3 @@
 # Ticket notes

-Notes about the support tickets that the help desk receives.
+Notes about support tickets for the help desk.

git push

To /Users/amira/projects/c14-lab/team-remote.git
   5a15d07..8b92dc4  add-ticket-104 -> add-ticket-104

Your hashes are different from 8b92dc4, because you made these commits.

As Amira, in ticket-notes. Fetch, then check the scope:

cd ~/projects/c14-lab/ticket-notes
git fetch
git log --oneline main..origin/add-ticket-104
git diff --stat main...origin/add-ticket-104

From /Users/amira/projects/c14-lab/team-remote
   5a15d07..8b92dc4  add-ticket-104 -> origin/add-ticket-104
8b92dc4 Undo the README wording change
27ce945 Remove scratch notes
95b27b0 Fix the format of ticket 104
5a15d07 Improve README wording
e34484f Add ticket 104
 tickets.txt | 1 +
 1 file changed, 1 insertion(+)

The PR has five commits now, but the total change is one line in tickets.txt. The fix commits cancel the README change and scratch.txt. Update the local branch, and run the check:

git switch add-ticket-104
git pull
sh check.sh
git switch main

Fast-forward
 README.md   | 2 +-
 scratch.txt | 2 --
 tickets.txt | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)
 delete mode 100644 scratch.txt
PASS: 4 ticket lines checked
Switched to branch 'main'
Your branch is up to date with 'origin/main'.

Verdict: approve. Every blocking comment is fixed, and the check passes.

One thing remains: commit e34484f still contains scratch.txt in the history of the branch. Here that is acceptable, because the notes are not secret. If the file contained a secret, removing it in a new commit would not be enough. See Keep the repository clean. The next lesson shows a merge method that keeps the five commits out of main.

Common mistake

The diff shows changes that the author never made

Symptom: you use two dots with git diff. The output shows a file that the PR does not touch, with a line removed:

git diff --stat main..origin/add-ticket-104

 README.md    | 2 +-
 contacts.txt | 1 -
 scratch.txt  | 2 ++
 tickets.txt  | 1 +
 4 files changed, 4 insertions(+), 2 deletions(-)

git diff main..origin/add-ticket-104 -- contacts.txt

diff --git a/contacts.txt b/contacts.txt
index 9563930..0ce0116 100644
--- a/contacts.txt
+++ b/contacts.txt
@@ -1,4 +1,3 @@
 Help desk: ext. 4100
 Network team: ext. 4410
 Accounts team: ext. 4300
-On-call engineer: 555-0100

It looks like Sam deleted the on-call number. He did not.

Cause: two dots compare the newest commit of main with the newest commit of the branch. main got a new commit after Sam made his branch. The branch does not have it, so the diff shows that commit in reverse. To see the commit, list what main has and the branch does not:

git log --oneline origin/add-ticket-104..main

2e9e37c Add the on-call number

Fix: use three dots for a review diff: git diff main...origin/add-ticket-104. Three dots compare the branch with the commit where it started, so you see only the changes of the PR. If you already wrote a comment about the deleted line, remove it.

Common mistake

The author says it is fixed, but the check still fails

Symptom: Sam pushed a fix. You switch to add-ticket-104 and run the check. It still fails on the old line, and git status says that you are up to date:

Switched to branch 'add-ticket-104'
Your branch is up to date with 'origin/add-ticket-104'.
FAIL: these lines in tickets.txt do not start with 'Ticket <number>: '
4:ticket 104 - VPN drops every hour.

Cause: you did not fetch. “Up to date” compares with your record of the remote, and that record is old. You are reviewing the old version of the PR.

Fix:

Run git fetch. The output shows add-ticket-104 -> origin/add-ticket-104 with two hashes.
Run git status. It now says Your branch is behind 'origin/add-ticket-104'.
Run git pull, then sh check.sh again.

Fetch at the start of every review, and again before each new look at a PR.

Check your understanding

Explain: Amira runs git diff main..origin/add-ticket-104. Why does the output show '-On-call engineer: 555-0100', when Sam never changed contacts.txt? What must she run instead?

Show the answer

Two dots compare the two newest commits directly. main has a commit, 2e9e37c Add the on-call number, that was made after the branch started. The branch does not have that line, so the comparison shows it as removed. She must use three dots: git diff main...origin/add-ticket-104. It compares the branch with its merge base, and shows only the changes that the PR makes.

Check your understanding

Practical check: a teammate asks you to review fix-contacts → main. Give the commands, in order, that show the commits, the changed files, the full change, and the result of the check.

Show the answer

git fetch, so that you review the newest version.
git log --oneline main..origin/fix-contacts: the commits of the PR.
git diff --stat main...origin/fix-contacts: the files that it changes.
git diff main...origin/fix-contacts, and git show <hash> for each commit: the full change.
git switch fix-contacts, then sh check.sh: the check result on the branch. Then git switch main.

Then write comments marked as blocking or suggestion, and a verdict.

Recap

Review against a checklist: scope, behaviour, checks, accidental files, unrelated changes and messages.
Use git log main..<branch> for the commits of a PR, and git diff main...<branch> for its changes.
git show <hash> shows what one commit really changed. The message is not the change.
Run the check on the branch, not on main. Fetch first.
Write comments that are specific, marked as blocking or suggestion, and kind. End with a verdict.
The author fixes a PR with new commits on the same branch. Then you review it again.

Next: Merge strategies