Hacker News new | ask | show | jobs
by JoshTriplett 3734 days ago
If someone prepares a pull request with a well-structured series of commits, making a logical series of changes, where the project builds and passes tests after each commit, then those commits shouldn't get squashed.

However, I frequently see people adding more commits on top of a pull request to fix typos, or do incremental development, where only the final result builds and passes, but not the intermediate stages, and where the changes are scattered among the commits with no logical grouping. In that case, I'd rather see them squashed and merged than merged in their existing form, and having a button to do that makes it more likely to happen.

3 comments

The trick is to not squash everything into one giant commit, but to use rebase -i liberally to squash/fixup those typo fix commits where they belong.
That's what the author of the pull request should do. But this provides a potentially useful alternative when that doesn't happen.
Plain squashing commits, while still a valid option in very few cases, will likely lead to gigantic commits that are hard to reason about.

I've seen projects where maintainers clean up poor commits before merging them: rebase/squash/reword only what's appropriate.

It's also the case that you lose the code review if you force push to a PR's branch after adding in a typo fix and squashing locally, right?

That's a pretty good reason not to squash till the review is done.

> It's also the case that you lose the code review if you force push to a PR's branch after adding in a typo fix and squashing locally, right?

Not as far as I can tell; I've force-pushed pull request branches many times, and the code reviews seem to stick around. (Perhaps they wouldn't if the code changed more drastically, like files disappearing; I haven't tried that.)