You’ve spent an afternoon on a ticket that says “free shipping should start at 50, not above 50.” Along the way you noticed totals were printed as 19.9 instead of 19.90, and you renamed a variable you didn’t like. Now git diff shows all three in one file, and it’s tempting to commit them as “Fix shipping and cleanup.”
The question “what makes a good commit?” comes up in interviews more than people expect, often phrased as “how do you structure your commits in a pull request?” Interviewers aren’t after style rules. They want to know whether you understand what history is used for: reviewing, reverting, bisecting, backporting and explaining why a line exists.
One logical change, and it still works
An atomic commit has two properties. It contains one logical change, so its message can describe it in one sentence without “and.” And the project builds and its tests pass at that commit, not just at the end of the branch.
“One logical change” isn’t the same as “small.” Renaming a function across 80 files is one logical change, and it should be one commit even though it’s large, because splitting it would leave commits where the code doesn’t compile. Conversely, a five-line diff that changes a business rule and also reformats a log message is two changes.
The “still works” half is the one people forget. A branch whose commits go “add call to new function,” then “add new function” builds fine at the tip and fails at the first commit. Anyone who bisects through it later lands on a commit that can’t be tested.
Splitting a mixed diff with git add -p
Here’s the situation from the opening, in a small demo repository. One hunk changes the shipping rule, the other changes formatting:
@@ -1,8 +1,8 @@
export function shippingCost(order) {
- if (order.total > 50) return 0;
+ if (order.total >= 50) return 0;
return 4.99;
}
-export function formatOrder(o) {
- return `#${o.id} ${o.total}`;
+export function formatOrder(order) {
+ return `#${order.id} ${order.total.toFixed(2)}`;
}git add -p walks through the changes and asks about each hunk. Git grouped both edits into one hunk because they’re only a few lines apart, so the first answer is s, to split it:
$ git add -p orders.js
(1/1) Stage this hunk [y,n,q,a,d,s,e,p,P,?]? s
Split into 2 hunks.
@@ -1,5 +1,5 @@
export function shippingCost(order) {
- if (order.total > 50) return 0;
+ if (order.total >= 50) return 0;
return 4.99;
}
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? y
@@ -3,6 +3,6 @@
...
(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? nNow git diff --cached shows only the shipping change and git diff shows only the formatting change. Commit them separately:
$ git commit -m "Make free shipping start at exactly 50"
$ git commit -am "Show order totals with two decimals"
$ git log --oneline
520af5a Show order totals with two decimals
5f5891d Make free shipping start at exactly 50
251ecca Add ordersWhen two changes sit on the same line, s can’t separate them. The e option opens the hunk in your editor so you can choose individual lines. If that gets fiddly, it’s often easier to undo one change by hand, commit, then redo it.
A caution: staging part of a file means the commit contains a version of the file you never ran. git stash push --keep-index sets aside the unstaged changes so the working tree matches what you’re about to commit, and you can run the tests on exactly that. git stash pop brings the rest back.
Fixing an earlier commit without a “fix review comments” commit
A reviewer points out that the shipping change has no test. If you add the test as a third commit called “address review,” the history now says the rule changed in one commit and was verified in another, unrelated-looking one. Git has a better path:
$ git add orders.test.js
$ git commit --fixup 5f5891d
[main 40c035c] fixup! Make free shipping start at exactly 50
1 file changed, 4 insertions(+)
create mode 100644 orders.test.jsThe fixup! prefix records which commit this belongs to. Before merging, an autosquash rebase moves it next to its target and melds them together:
$ git rebase -i --autosquash 251ecca
Successfully rebased and updated refs/heads/main.
$ git log --oneline --stat
1c7feda Show order totals with two decimals
orders.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
6e8a1fa Make free shipping start at exactly 50
orders.js | 2 +-
orders.test.js | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
251ecca Add orders
orders.js | 8 ++++++++
1 file changed, 8 insertions(+)(Running it with GIT_SEQUENCE_EDITOR=: accepts the generated plan without opening an editor, which is how the output above was produced.) The test now lives in the commit that changed the rule. Rebasing rewrites those commits, so do it on your own branch before it merges, not on shared history. Some teams prefer to keep review commits visible during review and squash or autosquash only at the end. GitHub’s “Squash and merge” goes further and turns the whole PR into one commit, which is fine when the PR is itself one logical change and loses information when it isn’t.
Where atomic history pays off
The benefit shows up months later, usually under pressure.
Revert. Suppose finance decides the 50 threshold was wrong after all. Reverting the atomic commit undoes exactly the rule and its test, and leaves the formatting improvement alone:
$ git revert --no-edit 6e8a1fa
Auto-merging orders.js
[main b2e512f] Revert "Make free shipping start at exactly 50"
Date: Thu Oct 1 10:00:00 2026 +0000
2 files changed, 1 insertion(+), 5 deletions(-)
delete mode 100644 orders.test.jsAfter this, orders.js has order.total > 50 again but keeps toFixed(2). With a combined “Fix shipping and cleanup” commit you’d have to revert both and redo the formatting by hand, or edit the revert.
Bisect. When every commit builds and does one thing, git bisect lands on a commit whose message tells you what changed. A 2,000-line “WIP” commit as the answer just tells you to start reading.
Cherry-pick. Backporting the shipping fix to a release branch is one clean pick. Mixed commits drag unrelated changes along.
Blame. git blame on the >= line points at a commit titled “Make free shipping start at exactly 50,” with a body that can explain why. That’s documentation that can’t go stale, because it’s attached to the exact line.
Writing the message
A subject line under about 50 characters, in the imperative (“Make free shipping start at exactly 50”, not “Made” or “Fixes”), then a blank line, then a body explaining why. The diff already shows what changed. The body is for the reason (“Marketing promised free shipping ‘from 50’; orders of exactly 50.00 were being charged”), any trade-offs, and a ticket reference. A useful test is whether someone reading only git log in a year could tell why this commit exists.
Interview practice
You’re reviewing a pull request with a single commit titled “Payment updates.” It changes the retry policy for card payments from 3 attempts to 5, upgrades the HTTP client library, reformats 40 files with a new Prettier config and fixes a null check that was crashing refunds. The author says splitting it would take too long. How would you respond, and how would you suggest they split it?
Answer
The concern isn’t neatness. These four changes have different risks and different reasons to be undone. If the client library upgrade causes timeouts in production, the team will want to revert it alone, without also reverting the refund crash fix. If someone bisects a refund bug later, the formatting change will make the diff unreadable. And the null-check fix probably needs backporting to a release branch, which is a clean cherry-pick only if it’s its own commit.
A practical order, each commit building and passing tests: first, the Prettier reformat on its own, with no logic changes, so reviewers can skim it (and the team can add it to .git-blame-ignore-revs so blame skips it). Second, the null-check fix with a test that reproduces the crash. Third, the library upgrade. Fourth, the retry change with its reasoning in the body. To get there from the existing commit, git reset HEAD~1 keeps all changes in the working tree, then git add -p and per-file git add build each commit in turn, using git stash push --keep-index to run tests before each one. It might take twenty minutes, and it makes every later revert, backport and investigation cheaper.