by philwelch 3 days ago
Squashing is fine if you’re just making a mess of temporary commits as you work and you don’t want to keep any of those changes separate in master, but that’s not a useful review workflow. A lot of times I’ve built a feature in a way that decomposed naturally into e.g. two commits: one to do a preparatory refactor (which might have a lot of noisy and repetitive changes, like changing a function signature) and another to actually change the behavior. You want those changes to be separate because it makes the changes easier to review; the reviewer quickly skims the first commit, observes that it’s a mechanical refactor, and the change in behavior has its own, smaller commit without all the noise.
“What if there’s feedback and you need to make changes after the code review?” Then I do the same thing I did before I posted the code review: make separate “fixup” commits and do an interactive rebase to squash them into my commits. (And yes, I do validate that the intermediate commits build cleanly.)
There’s nothing you get from stacked PR’s that you don’t also get from saying “please review my feature branch commit by commit”.
Yes what you’re describing is literally the thing GitHub has built but instead of having to make a bunch of compromises, there is dedicated UI and product metaphor for it.
Some examples of compromises:
You can’t merge partially merge a large “review commit by commit” PR so you are forced to wait until it is all ready to merge.
> You can’t merge partially merge a large “review commit by commit” PR so you are forced to wait until it is all ready to merge.
These are two different use cases. I thought we were talking about the one where a set of changes is more readable commit by commit but you still want to merge the whole set of changes, not the one where the change is too big to review and merge at once so you have to break it up into multiple reviews. The latter use case is more rare—frankly, it’s a bit of a red flag otherwise—and wasn’t difficult anyway.
Microsoft didn’t need to build anything because it was already built into Git. The only problem is, if people knew how to use Git, Microsoft couldn’t lock them into a proprietary version control platform.