4 ms·

> But there are so many people into it, that I feel I must be missing something obvious and it bothers me. As someone in favor of squashing, I can say that I d

> But there are so many people into it, that I feel I must be missing something obvious and it bothers me. As someone in favor of squashing, I can say that I don't want to see things like "oops, reverting last commit" popup in my git history, especially if I'm browsing history or bisecting a bug. That's noise - useless data. OTOH, commits should be the Minimum Necessary Change to accomplish a well-defined goal. The code itself should always be clear on what it is doing, otherwise it's badly written. If it's "deep magic", then comment it in the code as such. That being said, I do think that reasoning for why a change was made, at every level, should be in the commit message. I'm also not a fan of merge, but prefer squash+rebase. In all cases/workflows, it can be abused, and people writing bad code, bad comments or bad commit messages will do so until you can make them care to do it better. There's no silver bullet.

JimDabell 10y ago
> As someone in favor of squashing, I can say that I don't want to see things like "oops, reverting last commit" popup in my git history There's a middle ground between squashing and leaving a load of disorganised crap in the history. Rebase before merging. It gives you a chance to clean up the rubbish, but it doesn't force you to squash an entire feature's work into a single commit. You can preserve the logical changes without letting the crap into your history.
- ghayes 10y ago
  I agree here. Be a good steward of your commits, and let `rebase -i` be your tool for that. A bunch of "err, try this instead" should be washed away, but it doesn't help to commit "Add Huge Feature" +10,000/-2,000 because "I should squash my feature"
  - mikepurvis 10y ago
    If you're putting up 10k lines of code in a single review, you're doing it wrong anyway. Why wouldn't you have multiple reviews that each add different parts of a feature and link to the same ticket?
    - adrianratnapala 10y ago
      If I wanted to review 200 lines of changes, then I would still prefer to see it as a series of patches, each maybe with 1-20 lines changed and each with a clear purpose.
_ikke_ 10y ago
That's what things like git rebase are for (nice workflow with git commit --fixup + git rebase -i --autosquash). Then you clean up your fixes, while still keeping valuable history. With squash merges you are throwing out the baby with the bathwater.
JoshTriplett 10y ago
You should definitely squash away fixes to commits that are part of the same commit series you're submitting; you don't want to submit a broken patch followed by a second patch to fix that broken patch. But as part of your point that commits should be the "minimum necessary change", it makes sense to send a logical series of patches that iteratively change a project to implement a new feature (e.g. adding new infrastructure), rather than one big patch to implement that feature.
hobarrera 10y ago
> The code itself should always be clear on what it is doing Yes, but WHY it does that may not be obvious. "Oh, this code checks for a funny condition, but why would we care about that?". That's what commit messages are for.

JimDabell 10y ago

> As someone in favor of squashing, I can say that I don't want to see things like "oops, reverting last commit" popup in my git history There's a middle ground between squashing and leaving a load of disorganised crap in the history. Rebase before merging. It gives you a chance to clean up the rubbish, but it doesn't force you to squash an entire feature's work into a single commit. You can preserve the logical changes without letting the crap into your history.

ghayes 10y ago
I agree here. Be a good steward of your commits, and let `rebase -i` be your tool for that. A bunch of "err, try this instead" should be washed away, but it doesn't help to commit "Add Huge Feature" +10,000/-2,000 because "I should squash my feature"
- mikepurvis 10y ago
  If you're putting up 10k lines of code in a single review, you're doing it wrong anyway. Why wouldn't you have multiple reviews that each add different parts of a feature and link to the same ticket?
  - adrianratnapala 10y ago
    If I wanted to review 200 lines of changes, then I would still prefer to see it as a series of patches, each maybe with 1-20 lines changed and each with a clear purpose.

_ikke_ 10y ago

That's what things like git rebase are for (nice workflow with git commit --fixup + git rebase -i --autosquash). Then you clean up your fixes, while still keeping valuable history. With squash merges you are throwing out the baby with the bathwater.

JoshTriplett 10y ago

You should definitely squash away fixes to commits that are part of the same commit series you're submitting; you don't want to submit a broken patch followed by a second patch to fix that broken patch. But as part of your point that commits should be the "minimum necessary change", it makes sense to send a logical series of patches that iteratively change a project to implement a new feature (e.g. adding new infrastructure), rather than one big patch to implement that feature.

hobarrera 10y ago

> The code itself should always be clear on what it is doing Yes, but WHY it does that may not be obvious. "Oh, this code checks for a funny condition, but why would we care about that?". That's what commit messages are for.