5 ms·

What's the deal with squashing commits anyways? I'm genuinely asking, because I've only worked with "squash everything before you put it up for review" but have

by alexb_ 4y ago

What's the deal with squashing commits anyways? I'm genuinely asking, because I've only worked with "squash everything before you put it up for review" but have never really figured out why past "it's what we've always done".

softwaredoug 4y ago
I think people will have a branch with like 40 dumb commits like "fix typo" but when you merge it, people don't want to go through all those 40 commits in main. They want to instead read "Refactored frob reactor to use glom framework".
- planede 4y ago
  merge commit with git log --first-parent is alright.
fs111 4y ago
fix typo yet another typo test fix aaah, why is this test failing? merge foobar/narf revert foo foo Is just not a good history to preserve
- vultour 4y ago
  Boggles my mind how so many people in this thread are against rebasing. Have they never needed to remove these commits? I do that pretty much daily, especially when troubleshooting something in a CI pipeline.
  - Zitrax 4y ago
    I prefer rebasing too, but merging doesn't mean that you have to merge in all noisy minor fixups, you can combine the strategies and rebase the feature branch into a set of sensible commits, then merge that branch. Preferably even with a CI that checks each commit before merge such that it will not cause issues later during eventual bisections. --fixup and --autosquash are handy for such workflows.
    - __blockcipher__ 4y ago
      Using interactive rebase in that way is a great tool, but we're talking about people that are against rebasing in general, so it's not the most relevant point IMO. i.e. the anti-rebase people are usually of the "never rebase" flavor. Because they're also the "history is sacred even though I never look at history because if I did I would get dizzy and throw up" crowd :P
- deleted 4y ago
  [deleted]
msingh_5 4y ago
So that atomic changes are consolidated in a single commit/PR. Sometimes, the flow for a single feature A: - commit change AA - commit change AB - fix change in AA - extend AB In this case, until merged into main/master and to reduce noise for the reviewer (especially if using a code review each commit type process), its best to squash all commits into a single "feature A".
3np 4y ago
So you can get a semantic and useful history before submitting a patch/PR for review (or before merge). Doing hacky and messy WIP commits is totally fine. Just clean them up before having others read them. In case you're not aware: git rebase --interactive [remote/]ref
eru 4y ago
So there's (at least) two uses for commits. The first is to keep a log of what you are working on. For me, that's lots of small and dumb commits. The second is to provide a story for review. Most of the time, when the change you are putting out for review is small and simple, you can just put it all into a single commit. Sometimes, your change is more complicated and it makes sense to break it into a series of related commits. Along the line of 'first do the refactor that makes the complicated change simple, then make the simple change'. Your job as an author is to hide the ugly reality of how you actually came up with the change, and present the reviewer a sanitised view of reality. Reviewing code is hard, so it's best to make it as easy as possible.
- wackget 4y ago
  Why wouldn't you use a local branch (or something?) for your ugly commits and then merge everything from that branch into the main shared branch when you're done?
  - eru 4y ago
    Because that's hard to review. I am doing a lot of exploratory programming and way point commits. I don't need the reviewer to understand all the mistakes I made and bad designs I considered. It's enough work to understand the finished design. As a rule of thumb, every commit that lands in master's history should build and pass tests. So that eg 'git bisect' works. But it's not a good idea to put that same requirement on waypoint commits I make along the way, when exploring.
  - __blockcipher__ 4y ago
    Because then you're going to merge in the ugly commits and make everyone who needs to look at the history in the future have to work that much harder to understand what's going on.
    - eru 4y ago
      And you aren't just making the human's job harder, but also tools like 'git bisect' work better when every commit in master builds and passes tests.
evandale 4y ago
In my experience the squashing crowd is much louder and cares more about squashing than the crowd who prefers to see every commit. I hate squashing, but I've been beaten down into doing it many times because in the end it doesn't actually matter. The merge commits everywhere argument falls apart if you use log --no-merges. The shitty commit messages argument is solved by not allowing shitty commit messages. The "fixed typo", "oops left TODO", "another typo" chain of commits argument is solved by telling people to not do a million commits like that and IMO those should be squashed. The clean history argument is solved when you don't allow the useless "fixing typo" commits in feature-xyz branches. Your history should be clean and contain a history of the progress that was made. But ultimately the easiest way to solve all these problems is to force everyone to squash. You only have to police people on a single commit and people don't have to learn about options like --no-merges.
- Izkata 4y ago
  > I hate squashing, but I've been beaten down into doing it many times because in the end it doesn't actually matter. For those of us on maintenance teams, who actually have to dig in to the history to figure out what happened, not squashing matters a lot.
  - evandale 4y ago
    You'll have to elaborate with specifics because in my experience it doesn't matter if there's 100 or 10000 commits - git bisect works great in both instances.
    - RadiozRadioz 4y ago
      Bisect will let you find the big squash commit that caused the regression, but lots of the information about why the change happened was lost during the squash. Often it's sufficient to just know the broad feature that the change was in aid of, but you're out of luck if you're diagnosing anything subtle. Bisect works better with smaller commits.
    - Izkata 4y ago
GuB-42 4y ago
There are essentially two approaches: - History is sacred and can't be changed. It typically goes with a merge-based workflow. The good thing is that nothing is lost, what you see is what really happened and you won't make a mess with inappropriate push --force. The downside is that it looks messy since you are going to see every typo and you will have some non functional code in your history. People using that workflow usually don't squash since squashing rewrites history. - History is like documentation and have to be nice and clean. It typically goes with a rebase-based workflow. This is what the article is about. The good thing is that every commit has working code, and git log can effectively replace a more formal change log. The downside is that you lose information and the git log doesn't represent what really happened, furthermore, since push --force is often used, your local branch may not be the branch you think you are on, you may even end up destroying other people commits. People using this workflow usually squash to make their commits nicer. I prefer the "history is sacred" workflow myself, but both options are valid.
- __blockcipher__ 4y ago
  It's ironic that the "history is sacred" crowd contains the people least likely to actually look at and use the git history to aid their contextual understanding of the codebase. Why do I say that? Well, because once you've come across a useless typo commit for the 12th time, you start to very quickly see how such commits needlessly bloat the history and make it far harder to understand (and make things like git blame much more unwieldy), while not actually offering any benefit. It doesn't matter to me at all 3 months later to know that you initially had a typo in your first commit and then fixed it the subsequent commit. That should just be a single commit. There are some very rare edge cases where having the whole messy history can offer some insight of how a certain mistake ended up being made, but IMO in 999/1000 cases that's not the case.
  - Arelius 4y ago
    Squashing and rebasing don’t fix the shitty commit message problem. Discipline and good teamwork do. All to often in rebase+squash heavy repos I start going through the history and find the mega-commit that introduced a change and gain zero insight. In comparison, merge heavy repos have a lot of merge this, or fix this typo commits, that are fairly easy to skip right over when not pertaining to your actual change, but prove invaluable when the problem was introduced by some weird bad conflict resolution including a typo, or countless other problems where just all context is otherwise lost. I think it’s unfair to characterize one group as not using the history, but instead represent differences in how groups use them. As an archaeologist vs daily change log perhaps is a better characterization?

by alexb_ 4y ago

softwaredoug 4y ago

I think people will have a branch with like 40 dumb commits like "fix typo" but when you merge it, people don't want to go through all those 40 commits in main. They want to instead read "Refactored frob reactor to use glom framework".

planede 4y ago
merge commit with git log --first-parent is alright.

fs111 4y ago

fix typo yet another typo test fix aaah, why is this test failing? merge foobar/narf revert foo foo Is just not a good history to preserve

vultour 4y ago
Boggles my mind how so many people in this thread are against rebasing. Have they never needed to remove these commits? I do that pretty much daily, especially when troubleshooting something in a CI pipeline.
- Zitrax 4y ago
  I prefer rebasing too, but merging doesn't mean that you have to merge in all noisy minor fixups, you can combine the strategies and rebase the feature branch into a set of sensible commits, then merge that branch. Preferably even with a CI that checks each commit before merge such that it will not cause issues later during eventual bisections. --fixup and --autosquash are handy for such workflows.
  - __blockcipher__ 4y ago
    Using interactive rebase in that way is a great tool, but we're talking about people that are against rebasing in general, so it's not the most relevant point IMO. i.e. the anti-rebase people are usually of the "never rebase" flavor. Because they're also the "history is sacred even though I never look at history because if I did I would get dizzy and throw up" crowd :P
deleted 4y ago
[deleted]

msingh_5 4y ago

So that atomic changes are consolidated in a single commit/PR. Sometimes, the flow for a single feature A: - commit change AA - commit change AB - fix change in AA - extend AB In this case, until merged into main/master and to reduce noise for the reviewer (especially if using a code review each commit type process), its best to squash all commits into a single "feature A".

3np 4y ago

So you can get a semantic and useful history before submitting a patch/PR for review (or before merge). Doing hacky and messy WIP commits is totally fine. Just clean them up before having others read them. In case you're not aware: git rebase --interactive [remote/]ref

eru 4y ago

So there's (at least) two uses for commits. The first is to keep a log of what you are working on. For me, that's lots of small and dumb commits. The second is to provide a story for review. Most of the time, when the change you are putting out for review is small and simple, you can just put it all into a single commit. Sometimes, your change is more complicated and it makes sense to break it into a series of related commits. Along the line of 'first do the refactor that makes the complicated change simple, then make the simple change'. Your job as an author is to hide the ugly reality of how you actually came up with the change, and present the reviewer a sanitised view of reality. Reviewing code is hard, so it's best to make it as easy as possible.

wackget 4y ago
Why wouldn't you use a local branch (or something?) for your ugly commits and then merge everything from that branch into the main shared branch when you're done?
- eru 4y ago
  Because that's hard to review. I am doing a lot of exploratory programming and way point commits. I don't need the reviewer to understand all the mistakes I made and bad designs I considered. It's enough work to understand the finished design. As a rule of thumb, every commit that lands in master's history should build and pass tests. So that eg 'git bisect' works. But it's not a good idea to put that same requirement on waypoint commits I make along the way, when exploring.
- __blockcipher__ 4y ago
  Because then you're going to merge in the ugly commits and make everyone who needs to look at the history in the future have to work that much harder to understand what's going on.
  - eru 4y ago
    And you aren't just making the human's job harder, but also tools like 'git bisect' work better when every commit in master builds and passes tests.

evandale 4y ago

In my experience the squashing crowd is much louder and cares more about squashing than the crowd who prefers to see every commit. I hate squashing, but I've been beaten down into doing it many times because in the end it doesn't actually matter. The merge commits everywhere argument falls apart if you use log --no-merges. The shitty commit messages argument is solved by not allowing shitty commit messages. The "fixed typo", "oops left TODO", "another typo" chain of commits argument is solved by telling people to not do a million commits like that and IMO those should be squashed. The clean history argument is solved when you don't allow the useless "fixing typo" commits in feature-xyz branches. Your history should be clean and contain a history of the progress that was made. But ultimately the easiest way to solve all these problems is to force everyone to squash. You only have to police people on a single commit and people don't have to learn about options like --no-merges.

Izkata 4y ago
> I hate squashing, but I've been beaten down into doing it many times because in the end it doesn't actually matter. For those of us on maintenance teams, who actually have to dig in to the history to figure out what happened, not squashing matters a lot.
- evandale 4y ago
  You'll have to elaborate with specifics because in my experience it doesn't matter if there's 100 or 10000 commits - git bisect works great in both instances.
  - RadiozRadioz 4y ago
    Bisect will let you find the big squash commit that caused the regression, but lots of the information about why the change happened was lost during the squash. Often it's sufficient to just know the broad feature that the change was in aid of, but you're out of luck if you're diagnosing anything subtle. Bisect works better with smaller commits.
  - Izkata 4y ago

GuB-42 4y ago

There are essentially two approaches: - History is sacred and can't be changed. It typically goes with a merge-based workflow. The good thing is that nothing is lost, what you see is what really happened and you won't make a mess with inappropriate push --force. The downside is that it looks messy since you are going to see every typo and you will have some non functional code in your history. People using that workflow usually don't squash since squashing rewrites history. - History is like documentation and have to be nice and clean. It typically goes with a rebase-based workflow. This is what the article is about. The good thing is that every commit has working code, and git log can effectively replace a more formal change log. The downside is that you lose information and the git log doesn't represent what really happened, furthermore, since push --force is often used, your local branch may not be the branch you think you are on, you may even end up destroying other people commits. People using this workflow usually squash to make their commits nicer. I prefer the "history is sacred" workflow myself, but both options are valid.

__blockcipher__ 4y ago
It's ironic that the "history is sacred" crowd contains the people least likely to actually look at and use the git history to aid their contextual understanding of the codebase. Why do I say that? Well, because once you've come across a useless typo commit for the 12th time, you start to very quickly see how such commits needlessly bloat the history and make it far harder to understand (and make things like git blame much more unwieldy), while not actually offering any benefit. It doesn't matter to me at all 3 months later to know that you initially had a typo in your first commit and then fixed it the subsequent commit. That should just be a single commit. There are some very rare edge cases where having the whole messy history can offer some insight of how a certain mistake ended up being made, but IMO in 999/1000 cases that's not the case.
- Arelius 4y ago
  Squashing and rebasing don’t fix the shitty commit message problem. Discipline and good teamwork do. All to often in rebase+squash heavy repos I start going through the history and find the mega-commit that introduced a change and gain zero insight. In comparison, merge heavy repos have a lot of merge this, or fix this typo commits, that are fairly easy to skip right over when not pertaining to your actual change, but prove invaluable when the problem was introduced by some weird bad conflict resolution including a typo, or countless other problems where just all context is otherwise lost. I think it’s unfair to characterize one group as not using the history, but instead represent differences in how groups use them. As an archaeologist vs daily change log perhaps is a better characterization?