My ideal workflow is commits are as small as possible and PRs "tell a story", meaning that they provide the context for a commit.
I will split up a PR into
- Individual steps a a refactor, especially making any moves their own commits
- I add tests *before* the feature (passing, showing the old behavior)
- The actual fix or feature commit is tiny, with the diff of the tests just demontstrating how behavior changed
This makes it really fast to review a PR because you can see the motivation while only looking at a small change. No jumping around trying to figure out how the pieces fit together.
The main reason I might split up a PR like that is if one piece is likely to generate a discussion before its merged. I then make that a separate PR and as small as possible so we can have a focused discussion.
As someone who has often had to dig into the history to figure out what happened, I always want to see at least this. And I wouldn't be opposed to seeing it broken down even more as it was worked on. Not one big squash merge that hides what really happened.
I'll also add one more to your list: Any improvements that came out of the review but stayed in that merge should each be individual commits. I've seen hard-to-trigger bugs get introduced from what should have been just a style improvement.
One of the problems is that GitHub's UI and workflow isn't very good for this in various ways (can't review commits, can't really diff with previous after amending commit).
So as a rule, I tend to stick with "1 PR == 1 commit", except when there's a compelling reason not to.
I will split up a PR into
- Individual steps a a refactor, especially making any moves their own commits
- I add tests *before* the feature (passing, showing the old behavior)
- The actual fix or feature commit is tiny, with the diff of the tests just demontstrating how behavior changed
This makes it really fast to review a PR because you can see the motivation while only looking at a small change. No jumping around trying to figure out how the pieces fit together.
The main reason I might split up a PR like that is if one piece is likely to generate a discussion before its merged. I then make that a separate PR and as small as possible so we can have a focused discussion.
I hate squash merges.