[jcoder]

Not the author, but when I use the phrase I mean each commit accomplishes a single important thing, but also that each commit is complete: it includes necessary tests for example. IMO every commit that lands on `main` must pass the test suite (this means intermediate commits should be squashed into that atomic commit).

[epage]

The more I've been doing open source maintenance and contributions where there isn't as much context between the code author and reviewer, the more I've been pushing for a little more than this.

- Add tests in a commit *before* the fix. They should pass, showing the behavior before your change. Then, the commit with your change will update the tests. The diff between these commits represents the change in behavior. This helps the author test their tests (I've written tests thinking they covered the relevant case but didn't), the reviewer to more precisely see the change in behavior and comment on it, and the wider community to understand what the PR description is about.

- Where reasonable, find ways to split code changes out of feature / fix commits into refactor commits. Reading a diff top-down doesn't tell you anything; you need to jump around a lot to see how the parts interact. By splitting it up, you can more quickly understand each piece and the series of commits tells a story of how the feature of fix came to be.

- Commits are atomic while PRs tell a story, as long as it doesn't get too big. Refactor are usually leading towards a goal and having them tied together with that goal helps to provide the context to understand it all. However, this has to be balanced with the fact that larger reviews mean more things are missed on each pass and its different things on each pass, causing a lot of "20 rounds of feedback in and I just noticed this major problem".

As an example of these is a recent PR of mine against Cargo: https://github.com/rust-lang/cargo/pull/14239

In particular, the refactors leading up to the final change made it so the actual fix was a one line change. It also linked out to the prior refactors that I split out into separate PRs to keep this one smaller.

[OJFord]

I agree, but I usually explain (and do) this from the side of fixing a bug, but where the test suite is currently passing: first commit adds the failing test (shows that it would have caught the error), second commit makes it pass.

Also agree with GP that each commit on master should be passing/deployable/etc., but I don't see why they can't be merge commits of a branch that wasn't like that.

[simonw]

I absolutely love that testing suggestion - I'd never considered shipping a whole separate commit adding the OLD test first, but having a second commit that then updates that test to illustrate the change in behavior is such an obviously good idea.