Code Review with Git

The reviewability principle

In any project that does code review, the reviewer's time is the bottleneck. Your branch could be brilliant, but if it lands on the reviewer as a fifty-file pull request with one giant commit titled updates, you have asked them to reconstruct your thinking from scratch. They will procrastinate. When they finally read it, they will miss things. The review will take days.

A series of small, coherent commits, each with a message that explains what the commit does and why, points the reviewer through the change in the same order you discovered it. They can agree commit by commit, raise targeted questions, and finish in one sitting. That is what reviewability means: the same code, shaped so it costs less to read.

Commit hygiene before review

Most of the work of making a change reviewable happens before anyone else sees it. Three habits cover almost everything.

Each commit is a coherent change

One logical step per commit. A rename is a commit. Extracting a helper without changing behavior is a commit. The new behavior is a commit. The test that exercises it is a commit. The message explains what the commit does and, more importantly, why. The how is in the diff. See making good commits for the message conventions used throughout this knowledge base.

Interactive rebase to clean up

Real development is messy. You make a commit, find a typo, fix it. You try a path, give up, come back. By the time you are ready for review, your branch contains the journey, not the destination. git rebase -i lets you rewrite that branch into the story you actually want to tell: squash fixups into the commits they fix, reorder so each step builds on the previous, drop dead-end experiments. See rebasing for the mechanics.

Use --fixup during development, then autosquash before review

The cleanest workflow is to commit fixups against a specific earlier commit while you are working, then let Git squash them automatically right before you push. Mark a fixup with --fixup=<sha>:

# Half an hour into review prep you spot a bug in an earlier commit.
git log --oneline
# 8a1b2c3 Add parser tests
# 4d5e6f7 Implement parser   <- the bug lives here

git add parser.py
git commit --fixup=4d5e6f7

# Later, before opening the PR: collapse fixups into their targets.
git rebase -i --autosquash origin/main

The --autosquash flag pre-arranges the rebase todo list so each fixup! commit lands immediately after its target with the right action. You confirm the list and Git does the rest. Set git config --global rebase.autoSquash true to make it the default.

Reading what you are submitting

Before clicking "open pull request", read your own change as if someone else wrote it. Two commands do the heavy lifting.

# The commits your branch adds, oldest first.
git log --oneline --reverse origin/main..HEAD

# The cumulative diff your branch introduces, computed from the merge base.
git diff origin/main...HEAD

Notice the two-dot range in git log and the three-dot range in git diff. They mean different things, and the difference matters enough to deserve its own section.

Walk the log, then walk the diff. Ask the questions a reviewer will ask: Is each commit message true and complete? Do the changes match the messages? Are there debug prints, commented-out code, or stray files I forgot? Is there a commit that could be split? Is there a commit that should be squashed? You will catch real problems every time.

Two-dot vs three-dot ranges

Git accepts two range syntaxes that look almost identical and mean different things. Which one is right depends on the command.

For day-to-day review prep:

• git log origin/main..HEAD — "the commits I am proposing".
• git diff origin/main...HEAD — "the net change my branch introduces, computed from where I branched off". This matches what GitHub or GitLab will show in the PR view, even if main has moved forward since you started.

Habits that make diffs reviewable

A few small disciplines reduce review churn out of all proportion to how much they cost.

Do not mix reformatting with logic changes

If a hunk of the diff is "reindented the whole function" plus "changed one branch in the middle", the reviewer has to scan every changed line to find the real change. Make the reformat its own commit — even better, its own PR — so the logic commit shows only the real change.

Separate rename commits

Git detects renames heuristically by comparing file content; the detection is good but not perfect, and it gets worse when you rename and edit a file in the same commit. A standalone "rename Foo to Bar" commit gives Git the cleanest possible signal and gives the reviewer a diff they can scroll past with confidence.

Resist the urge to reformat the whole file

If your editor reformatted a file on save before you touched it, undo the formatting and reapply it as a dedicated commit, or revert the file and configure your editor to leave it alone for this change. The drive-by reformat is the single most common review-killer.

Keep generated files out of regular commits

Compiled output, bundle artifacts, lockfile mass-updates, and snapshot test fixtures should either live outside the repository entirely (via .gitignore) or be regenerated by a script as their own commit. A reviewer cannot meaningfully review a 12,000-line bundle diff.

Stacked changes

Some features are genuinely too big for a single reviewable PR. The escape hatch is a stack: split the work into a chain of small branches, each branching off the previous one, each submitted as its own PR. They merge in order, top of stack down to bottom.

main
  feature/part-1   <- PR #101  (foundation: types and storage)
    feature/part-2 <- PR #102  (built on part-1: read path)
      feature/part-3 <- PR #103  (built on part-2: write path + UI)

Each PR is small enough to actually review. The stack lets you keep building while earlier parts are still in review, instead of accumulating a single 3,000-line behemoth that no one wants to start. The cost is rebasing: when part-1 changes during review, part-2 and part-3 have to be rebased onto the new tip. Tools like git-spice, Graphite, gh stack, and Sapling automate the bookkeeping, but the underlying technique is plain Git: branches off branches, rebases when the base moves.

Responding to review

You have feedback. You need to address it without losing the reviewer's place. There are two schools, and most teams pick a hybrid.

Add new commits on top

Each round of feedback becomes a new commit ("Address review: rename helper", "Address review: handle empty input"). The original commits stay untouched. The reviewer can see exactly what changed since they last looked — the new commits are the response. Common on GitHub, encouraged by GitHub's "view changes since your last review" feature.

Amend and force-with-lease

You edit the original commits with git commit --amend or git rebase -i, so the final history reads cleanly: each commit is the right size and shape, with no "address review" noise. You publish with git push --force-with-lease, which refuses to overwrite the remote if someone else has pushed in the meantime. The downside: the reviewer loses the "what changed since last review" diff unless the forge supports range-diff (GitHub does, since 2022).

The hybrid most teams settle on

Add commits during review for clarity, then squash and clean up just before merge — either by running git rebase -i yourself or by clicking "Squash and merge" on the forge. You get a clean final history without losing the round-by-round trail.

Inspection options that help reviewers

When the cumulative diff is hard to read, Git has several flags that change how the diff is rendered without changing the underlying change. Use them in your self-review; suggest them to reviewers when a diff is misleading by default.

# Highlight blocks of code that were moved rather than added/deleted.
# Invaluable when reviewing extraction refactors.
git diff --color-moved=zebra origin/main...HEAD

# Word-level diff: show changes inside lines, not just whole-line replacements.
# Best for prose, docs, and config tweaks.
git diff --word-diff=color origin/main...HEAD

# Ignore whitespace entirely when comparing.
# Useful when an unrelated reformat snuck in and you want the real change.
git diff -w origin/main...HEAD

# Catch whitespace errors before you push.
git diff --check

The --color-moved option is the standout. Without it, moving a 30-line block from one file to another shows as 30 deletions and 30 additions and looks identical to a rewrite. With --color-moved=zebra, Git paints the moved blocks in alternating tints so you can see at a glance that nothing inside them changed.

git range-diff for review rounds

The hardest case for a reviewer is "you rebased my branch and now I cannot tell what changed". That is what git range-diff is for. It compares two versions of a patch series and shows, commit by commit, how the series evolved.

# Before rebasing, tag the current tip so you can refer back to it.
git tag review-round-1

# ...do your rebase, fixups, amends...

# Now compare the old series to the new one, with origin/main as the base.
git range-diff origin/main review-round-1 HEAD

Output shows each commit from the old series paired with its closest match in the new series, plus the inter-commit diff. Commits that exist only on one side are flagged. It is the same idea as GitHub's "changes since your last review", but it works locally and across arbitrary rebases.

Common pitfalls

Worked examples

git fetch origin
git rebase origin/main           # land on the current main

git log --oneline origin/main..HEAD
# 11 commits, much noise

git rebase -i origin/main        # opens the todo list
# In the editor, change "pick" to "squash" (s) or "fixup" (f)
# for the noise commits; reorder lines to group related work.

git log --oneline origin/main..HEAD
# 3 clean commits, each a coherent step

git add parser.py
git commit --fixup=4d5e6f7        # creates "fixup! Implement parser"

git add tests/parser_test.py
git commit --fixup=8a1b2c3        # creates "fixup! Add parser tests"

git rebase -i --autosquash origin/main
# Todo list is pre-arranged. Save and exit.

git push --force-with-lease

git diff --color-moved=zebra origin/main...HEAD -- handler.py helpers/parsing.py

git fetch origin

# Wrong: includes the inverse of every change upstream made since you branched.
git diff origin/main..HEAD

# Right: diff from the merge base of origin/main and HEAD to HEAD.
git diff origin/main...HEAD