4 ms·

This actually gives me insight into why one contributor I worked with was crazy about squashing all commits into one when merging into main. I still think that'

by ehnto 4y ago

This actually gives me insight into why one contributor I worked with was crazy about squashing all commits into one when merging into main. I still think that's not good, because you loose the commit message history, and why have history at all if you squash it away. But maybe this was why they preferred it. I find commit history is important if you have a long running client project with evolving context, so you can figure out why something was changed, not just what.

nabogh 4y ago
You can have both. A merge has two parents so one parent is the main branch minus this squashed commit, and the other parent leads to the commits made during development. It's the default behaviour that I've seen everywhere.
- thriftwy 4y ago
  git bisect basically refuses to work with merge commits, so using those ruin bisectability. As well as linear log, cherry-pick and scores of other tools which do not have intelligent merges handling.
  - Karellen 4y ago
    > git bisect basically refuses to work with merge commits, so using those ruin bisectability. I find that quite surprising, given how I see `git bisect` referenced by the Linux community as useful for tracking down the commit that introduced a problem, and Linus uses merge commits (including `octopus merges`) very heavily.
ianlevesque 4y ago
This is one thing that the FAANG workflow gets right. Multiple stacked commits are great as long as each one is a reviewable unit that makes sense on its own, and they don't need to be squashed to merge to main. What you need to avoid are a stack of commits like "added some code today" and "fixed bugs" and "try #3". Learn to amend existing commits instead...
- plugin-baby 4y ago
  > What you need to avoid are a stack of commits like "added some code today" and "fixed bugs" and "try #3". That’s what you need to avoid in your mainline. Mandating what developers do in development branches is like having a dress code at work and then complaining about your colleague’s choice of underwear.
  - int0x80 4y ago
    Nobody mandates developers about their private repos and commits, nobody cares about that. This is about what gets merged into master or what gets submitted for review.
  - LaLaLand122 4y ago
    Nobody cares about the underwear... until people create a PR explicitly saying "look at my underwear".
    - ianlevesque 4y ago
      And GitHub makes it 1-click easy to make a pull request full of underwear so it happens all the time.
- nyanpasu64 4y ago
  Squashing commits loses history, rebasing messy commits pollutes the "first parent" history, merging messy commits is cleaner but can force bisectors to wade through dead ends (which may or may not be useful, and may be worth preserving in a permanent side branch not merged), and rewriting commit histories into atomic commits (what I do most of the time) is difficult work which can take as long as writing the features upfront, and if done wrong can produce non-building commits. Sadly I haven't found a good solution for the workload yet, but this article suggests `git rebase -i main --exec "make"`, which I'll be sure to try when I have a chance.
LaLaLand122 4y ago
All my commits build, so I can do a bisect and see the problem is... somewhere in this huge commit? It doesn't seem better to me than "the problem is in one of these two small commits, I don't know which one because they don't build".
- ParetoOptimal 4y ago
  The smaller commits are, the more useful they are to bisect.
- sulam 4y ago
  Large commits are an antipattern that should be reworked, in code review at worst.
tcoff91 4y ago
There’s no reason to squash commits to achieve this anymore. The —-first-parent option of git bisect makes it so that when you bisect, it won’t select commits from within the middle of a feature branch. It will only pick the commits that were truly part of main. That way you can easily find out which PR broke things. Git log also has the first parent option and that is really helpful for helping you cut through the noise and see the high level overview of the history without seeing every commit.
wirrbel 4y ago
Among the options merge commit , rebase and squash merge, rebasing is the problematic one if the commits on the branch are not carefully crafted. Classical merging allows for bisecting and retains historicy. But IMHO if swash merged lead to a situation that relevant history is lost the PR is too big.
jerf 4y ago
Yes, I consider bisectability the resolution of the "do you want a clean history or do you want a commit history with every little twitch of the developer in it?". A clean history is better, because the latter is not bisectable. On average the latter is always broken. Commits that make it into the "real" branches should only be made when all the tests pass. (Larger projects that can't run all the tests should have a local test suite of all the "fast" or "local" tests, or whatever it is that can be run, for fast testing prior to commit.) Temporary commits on your own branch is fine since you often need their functionality for other local reasons. However, squashing everything down isn't the only or best answer. One can also create a series of commits, each of which is individually passing all the tests, but isn't one big bang. Of course sometimes you have no choice, some refactorings just fundamentally involve huge atomic changes to the code base, but it's still better to try to break these things down into multiple steps if at all possible so you can come back later with a new test and bisect. I don't use bisect often, but when I do, it can save me days of effort trying to find problems, so it's worth it to maintain the discipline of keeping the tests passing with all commits. Plus it's already a net benefit; maybe you are a way better programmer than me but I find that my changes that couldn't possibly break anything else in the code base break other things in the code base in modules I wasn't expecting with some frequency. Keeping all the tests passing as much as possible is a critical component in making sure that I make what I think of as "monotonic forward progress". There are things more demoralizing that breaking half the code base every time I make the slightest change and it not being caught until QA or deployment... but making a habit of this is up there for sure. Definitely a great way to add some burnout to your life. I've seen multi-decade projects basically strangle themselves to death this way; nobody was willing to touch any of the core code because it had no testing coverage and every time they tried it broke everything, so they just lived with some real garbage code at the core of it.
grumbel 4y ago
One area where I find squashing especially problematic is file renames. If you rename a file in one commit and fix up the references to and from the file in a second, git can follow the rename, but one commit will fail to compile. If you squash it into one commit on the other side, git will have a very hard time figuring out that the file got renamed and is not a brand new file, as both the rename and the fixup happened at the same time.

by ehnto 4y ago

nabogh 4y ago

You can have both. A merge has two parents so one parent is the main branch minus this squashed commit, and the other parent leads to the commits made during development. It's the default behaviour that I've seen everywhere.

thriftwy 4y ago
git bisect basically refuses to work with merge commits, so using those ruin bisectability. As well as linear log, cherry-pick and scores of other tools which do not have intelligent merges handling.
- Karellen 4y ago
  > git bisect basically refuses to work with merge commits, so using those ruin bisectability. I find that quite surprising, given how I see `git bisect` referenced by the Linux community as useful for tracking down the commit that introduced a problem, and Linus uses merge commits (including `octopus merges`) very heavily.

ianlevesque 4y ago

This is one thing that the FAANG workflow gets right. Multiple stacked commits are great as long as each one is a reviewable unit that makes sense on its own, and they don't need to be squashed to merge to main. What you need to avoid are a stack of commits like "added some code today" and "fixed bugs" and "try #3". Learn to amend existing commits instead...

plugin-baby 4y ago
> What you need to avoid are a stack of commits like "added some code today" and "fixed bugs" and "try #3". That’s what you need to avoid in your mainline. Mandating what developers do in development branches is like having a dress code at work and then complaining about your colleague’s choice of underwear.
- int0x80 4y ago
  Nobody mandates developers about their private repos and commits, nobody cares about that. This is about what gets merged into master or what gets submitted for review.
- LaLaLand122 4y ago
  Nobody cares about the underwear... until people create a PR explicitly saying "look at my underwear".
  - ianlevesque 4y ago
    And GitHub makes it 1-click easy to make a pull request full of underwear so it happens all the time.
nyanpasu64 4y ago
Squashing commits loses history, rebasing messy commits pollutes the "first parent" history, merging messy commits is cleaner but can force bisectors to wade through dead ends (which may or may not be useful, and may be worth preserving in a permanent side branch not merged), and rewriting commit histories into atomic commits (what I do most of the time) is difficult work which can take as long as writing the features upfront, and if done wrong can produce non-building commits. Sadly I haven't found a good solution for the workload yet, but this article suggests `git rebase -i main --exec "make"`, which I'll be sure to try when I have a chance.

LaLaLand122 4y ago

All my commits build, so I can do a bisect and see the problem is... somewhere in this huge commit? It doesn't seem better to me than "the problem is in one of these two small commits, I don't know which one because they don't build".

ParetoOptimal 4y ago
The smaller commits are, the more useful they are to bisect.
sulam 4y ago
Large commits are an antipattern that should be reworked, in code review at worst.

tcoff91 4y ago

There’s no reason to squash commits to achieve this anymore. The —-first-parent option of git bisect makes it so that when you bisect, it won’t select commits from within the middle of a feature branch. It will only pick the commits that were truly part of main. That way you can easily find out which PR broke things. Git log also has the first parent option and that is really helpful for helping you cut through the noise and see the high level overview of the history without seeing every commit.

wirrbel 4y ago

Among the options merge commit , rebase and squash merge, rebasing is the problematic one if the commits on the branch are not carefully crafted. Classical merging allows for bisecting and retains historicy. But IMHO if swash merged lead to a situation that relevant history is lost the PR is too big.

jerf 4y ago

Yes, I consider bisectability the resolution of the "do you want a clean history or do you want a commit history with every little twitch of the developer in it?". A clean history is better, because the latter is not bisectable. On average the latter is always broken. Commits that make it into the "real" branches should only be made when all the tests pass. (Larger projects that can't run all the tests should have a local test suite of all the "fast" or "local" tests, or whatever it is that can be run, for fast testing prior to commit.) Temporary commits on your own branch is fine since you often need their functionality for other local reasons. However, squashing everything down isn't the only or best answer. One can also create a series of commits, each of which is individually passing all the tests, but isn't one big bang. Of course sometimes you have no choice, some refactorings just fundamentally involve huge atomic changes to the code base, but it's still better to try to break these things down into multiple steps if at all possible so you can come back later with a new test and bisect. I don't use bisect often, but when I do, it can save me days of effort trying to find problems, so it's worth it to maintain the discipline of keeping the tests passing with all commits. Plus it's already a net benefit; maybe you are a way better programmer than me but I find that my changes that couldn't possibly break anything else in the code base break other things in the code base in modules I wasn't expecting with some frequency. Keeping all the tests passing as much as possible is a critical component in making sure that I make what I think of as "monotonic forward progress". There are things more demoralizing that breaking half the code base every time I make the slightest change and it not being caught until QA or deployment... but making a habit of this is up there for sure. Definitely a great way to add some burnout to your life. I've seen multi-decade projects basically strangle themselves to death this way; nobody was willing to touch any of the core code because it had no testing coverage and every time they tried it broke everything, so they just lived with some real garbage code at the core of it.

grumbel 4y ago

One area where I find squashing especially problematic is file renames. If you rename a file in one commit and fix up the references to and from the file in a second, git can follow the rename, but one commit will fail to compile. If you squash it into one commit on the other side, git will have a very hard time figuring out that the file got renamed and is not a brand new file, as both the rename and the fixup happened at the same time.