8 ms·

I actually hate squash merge because of all the noise it adds. Sure, the commit graph looks nicer, but it come with a terrible loss of information when doing gi

I actually hate squash merge because of all the noise it adds. Sure, the commit graph looks nicer, but it come with a terrible loss of information when doing git blame. I'm a big proponent of rebase and squash if it helps to make a commit more coherent, but we use squash merges by default in the current project I'm working on, and I die a little bit each time I try to understand what changes were related to a line when tracking down a bug.

nerdponx 3y ago
This is the big one for me. destroying Commit information just to keep the graph tidy is a bad idea in my opinion. It would be better if Git provided better tools for filtering the log, e.g. providing some mechanism to elide commits from parents of any merge commit other than the 1st.
- diek 3y ago
  > destroying Commit information just to keep the graph tidy is a bad idea in my opinion The commit information I see when telling teams to squash their branches on merge is not valuable. * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" (note, the broken tests were broken by the changes in the PR) As soon as that PR is merged those commits are worthless. And there are branches with dozens of those "fixing X" commits that would otherwise pollute the commit graph.
  - lelandfe 3y ago
    Those commits would be the bathwater one casts out alongside the useful commits in using squash merges.
    - diek 3y ago
      If the useful commits are the "baby" in your bathwater analogy, all the useful information in those commits is in the squashed commit. This assumes a branch being merged in represents one logical change (a feature/bugfix/etc) that is "right sized" to be represented by one commit.
      - mablopoule 3y ago
        Yes, but now it's mixed with the bathwater, and now morph into another metaphor as it become the needle in the haystack. It's okay to have 'low information' commits one can easily ignore in your history, as long as the 'high information' ones stay readable and coherent.
  - gray_-_wolf 3y ago
    > * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" Things like this should not be standalone commits though, they should be incorporated into the previous branch by amending the original work. It takes some effort to have a useful git history, it does not just happen on its own.
    - lesuorac 3y ago
      Sounds like six vs half-dozen. Why does it matter if somebody amends vs squashes?
      - gray_-_wolf 3y ago
        It does not matter if you have one commit. If your change is split into few commits for increased readability, in that case it does matter. Do you really believe that if, for example, this change to btrfs filesystem https://lore.kernel.org/linux-btrfs/cover.1699470345.git.josef@toxicpanda.com/T/#t https://lore.kernel.org/linux-btrfs/cover.1699470345.git.jos... would be squashed, nothing of value would be lost?
    - noahtallen 3y ago
      You can very easily rewrite your commit message on GitHub when squash merging. Since the organizations I work exclusively use squash merge, I often just update the commit to be more valuable, listing the important changes it contains. (And of course the PR in GitHub will contain the commit history of the branch that was squashed, as well as any discussion.) IMO, this is a lot simpler and easier to do than rebasing your branch to have a flawless history.
    - sodapopcan 3y ago
      I rather strongly disagree here. Having whitespaces mucks up commit, causing you to lose focus of what's actually important. I have `git blame` aliased to `git blame -w` which ignores whitespace-only changes. You can also reblame when you come across this formatting commits.
  - trevor-e 3y ago
    Yep, intermediate commits on a branch tend to be completely worthless. I'd much rather have "git blame" point to the commit that contains the entire change together.
    - dieselgate 3y ago
      Agree strongly, it's nice in theory to view the intermediate commits but in practice have never needed to look at them
  - 26fingies 3y ago
    You can usually see that in whatever tool youre using anyway. Blame -> find the PR -> see commit history.
- gray_-_wolf 3y ago
  You mean like for example `git log --first-parent`?
  - nerdponx 3y ago
    TIL, thank you! Now I know for a fact that squash-mergers have no excuse and can brandish the man page at them.
    - WorldMaker 3y ago
      `--first-parent` also today works for blame and bisect.
    - Izkata 3y ago
      People in general just have no idea how much version control is able to do. For one example try running "git help log" and just tap page-down a few (dozen) times to get an idea what's in there. For another example, you know how people hate aligning code vertically so much that linters don't allow it nowadays, the primary reason being that if you have to change the spacing then the diffs will identify far too many lines as having changed? Both git and svn have options to ignore whitespace changes: git diff -w svn diff -x -w
johnsbrayton 3y ago
I do squash merges but keep the feature branches. So after determining that I made a change as part of a big pull request, I can then look at the commit/blame history for the pull request source branch if necessary.
- matijsvzuijlen 3y ago
  This means having to keep these branches around cluttering up everything, and makes git bisect a lot more complicated.
- mvdtnz 3y ago
  I'm guessing you don't work on large projects. This would create an outrageous amount of noise in a busy repository.
skybrian 3y ago
This all depends on the project. Sometimes you don’t look at history all that much. Sometimes the loss of information is acceptable.
- Izkata 3y ago
  If you don't look at history much, why would you care about keeping it "clean"? Just keep the truth of the changes in the history for those of us who do use it, and you can continue ignoring it.
diek 3y ago
> I die a little bit each time I try to understand what changes were related to a line when tracking down a bug A change/feature/bug is a branch, which is squashed into a commit on your main branch, right? So your main branch should be a linear history of changes, one change per commit. How does that impact the ability to git blame?
- joshribakoff 3y ago
  Because sometimes a PR touches more code than a single commit, and you lose the more granular context surrounding the more granular changes. You can always ask git to make the log more coarse, but once you “destroy” the granular history it is for all intents and purposes gone.
- js2 3y ago
  Because unless it's the most trivial of features, you'll break it up into smaller commits which each explain what they are doing and make reviewing the change easier. As a simple example, I recently needed to update a json document that was a list of objects. I needed to add a new key/value to each object. The document had been hand edited over the years and had never been auto-formatted. My PR ended up being three commits: 1. Reformat the document with jq. Commit title explains it's a simple reformat of the document and that the next commit will add `.git-blame-ignore-revs` so that the history of the document isn't lost in `git blame` view. 2. Add `.git-blame-ignore-revs` with the commit ID of (1). 3. Finally, add the new key/value to each object. The PR then explains that a new key/value has been added, mentions that the document was reformatted through `jq` as part of the work, a recommends that the reviewer step through the commits to ignore the mechanical change made by (1). A followup PR added a pre-commit CI step to keep the document properly linted in the future.
  - diek 3y ago
    In general I agree with you, there are absolutely times where you want to retain commit history on a particular branch (although I try to keep the source tree from knowing about things like commit IDs). I would argue that those are by far the minority of PRs that I see. As I mentioned in another comment, _most_ PRs that I see have a ton of intermediary commits that are only useful for that branch/PR/review process (fixing tests, whitespace, etc). Generally the advice I give teams is, "squash by default" and then figure out where the exceptions to that rule are. That's mainly because, in my opinion, the downsides of a noisy commit graph filled with "addressing review comments" (or whatever) commits are a much bigger/frequent issue than the benefits you talk about. It really depends on the team.
WirelessGigabit 3y ago
Me too. I care a LOT about provenance. And squash merge completely breaks that. When my branch is up to date with `main` I can build an artifact, fast forward merge that branch into `main` and RETAIN the artifact, and merely update its tags to mark it as `merged` in. With a squash I lose that information. Now, GitHub does not allow me to do a fast-forward merge but I can still trace the 2 commits that are the parent of the resultant merge, and find the artifact based on that, and retag.
Lio 3y ago
Git blame confuses people even without squash merges. I've seen people forget to go back more than one commit and then blame the person who last indented a file instead of going back to the commit that actually wrote the code many times.
- erik_seaberg 3y ago
  We shouldn’t tamper with code we don’t actually need to fix, it’s not a good use of time and it makes history less useful. Just because it doesn’t look like I wrote it doesn’t make it wrong.
  - Lio 3y ago
    I’m thinking of situations where the surrounding structure of the code has been changed to correct a problem. That’s done by an automated tool. Correction of indentation is just a byproduct. I don’t consider that “tampering”.
    - deleted 3y ago
      [deleted]
    - WorldMaker 3y ago
      You can deal with that through tooling. In a lot of my work I call those types of automated tool commits "wrench" commits personally and even have a simple shell script to help automate committing them. In my case I prefix the command line with a wrench emoji. At that point it's very obvious in git blame that if a line starts with a wrench it was last touched by an automated tool of some sort. You can also very easily at that point grep your git log for wrenches to dump commit hashes into a git-ignore-revs file and automate that part too so that those commits don't even show up in git blame at all.
- sodapopcan 3y ago
  > I've seen people forget to go back more than one commit and then blame the person who last indented a file instead of going back to the commit that actually wrote the code many times. I default my `git blame` to `git blame -w` which ignores whitespace commits. Though knowing how to jump back commits should be required knowledge.

nerdponx 3y ago

This is the big one for me. destroying Commit information just to keep the graph tidy is a bad idea in my opinion. It would be better if Git provided better tools for filtering the log, e.g. providing some mechanism to elide commits from parents of any merge commit other than the 1st.

diek 3y ago
> destroying Commit information just to keep the graph tidy is a bad idea in my opinion The commit information I see when telling teams to squash their branches on merge is not valuable. * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" (note, the broken tests were broken by the changes in the PR) As soon as that PR is merged those commits are worthless. And there are branches with dozens of those "fixing X" commits that would otherwise pollute the commit graph.
- lelandfe 3y ago
  Those commits would be the bathwater one casts out alongside the useful commits in using squash merges.
  - diek 3y ago
    If the useful commits are the "baby" in your bathwater analogy, all the useful information in those commits is in the squashed commit. This assumes a branch being merged in represents one logical change (a feature/bugfix/etc) that is "right sized" to be represented by one commit.
    - mablopoule 3y ago
      Yes, but now it's mixed with the bathwater, and now morph into another metaphor as it become the needle in the haystack. It's okay to have 'low information' commits one can easily ignore in your history, as long as the 'high information' ones stay readable and coherent.
- gray_-_wolf 3y ago
  > * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" Things like this should not be standalone commits though, they should be incorporated into the previous branch by amending the original work. It takes some effort to have a useful git history, it does not just happen on its own.
  - lesuorac 3y ago
    Sounds like six vs half-dozen. Why does it matter if somebody amends vs squashes?
    - gray_-_wolf 3y ago
      It does not matter if you have one commit. If your change is split into few commits for increased readability, in that case it does matter. Do you really believe that if, for example, this change to btrfs filesystem https://lore.kernel.org/linux-btrfs/cover.1699470345.git.josef@toxicpanda.com/T/#t https://lore.kernel.org/linux-btrfs/cover.1699470345.git.jos... would be squashed, nothing of value would be lost?
  - noahtallen 3y ago
    You can very easily rewrite your commit message on GitHub when squash merging. Since the organizations I work exclusively use squash merge, I often just update the commit to be more valuable, listing the important changes it contains. (And of course the PR in GitHub will contain the commit history of the branch that was squashed, as well as any discussion.) IMO, this is a lot simpler and easier to do than rebasing your branch to have a flawless history.
  - sodapopcan 3y ago
    I rather strongly disagree here. Having whitespaces mucks up commit, causing you to lose focus of what's actually important. I have `git blame` aliased to `git blame -w` which ignores whitespace-only changes. You can also reblame when you come across this formatting commits.
- trevor-e 3y ago
  Yep, intermediate commits on a branch tend to be completely worthless. I'd much rather have "git blame" point to the commit that contains the entire change together.
  - dieselgate 3y ago
    Agree strongly, it's nice in theory to view the intermediate commits but in practice have never needed to look at them
- 26fingies 3y ago
  You can usually see that in whatever tool youre using anyway. Blame -> find the PR -> see commit history.
gray_-_wolf 3y ago
You mean like for example `git log --first-parent`?
- nerdponx 3y ago
  TIL, thank you! Now I know for a fact that squash-mergers have no excuse and can brandish the man page at them.
  - WorldMaker 3y ago
    `--first-parent` also today works for blame and bisect.
  - Izkata 3y ago
    People in general just have no idea how much version control is able to do. For one example try running "git help log" and just tap page-down a few (dozen) times to get an idea what's in there. For another example, you know how people hate aligning code vertically so much that linters don't allow it nowadays, the primary reason being that if you have to change the spacing then the diffs will identify far too many lines as having changed? Both git and svn have options to ignore whitespace changes: git diff -w svn diff -x -w

johnsbrayton 3y ago

I do squash merges but keep the feature branches. So after determining that I made a change as part of a big pull request, I can then look at the commit/blame history for the pull request source branch if necessary.

matijsvzuijlen 3y ago
This means having to keep these branches around cluttering up everything, and makes git bisect a lot more complicated.
mvdtnz 3y ago
I'm guessing you don't work on large projects. This would create an outrageous amount of noise in a busy repository.

skybrian 3y ago

This all depends on the project. Sometimes you don’t look at history all that much. Sometimes the loss of information is acceptable.

Izkata 3y ago
If you don't look at history much, why would you care about keeping it "clean"? Just keep the truth of the changes in the history for those of us who do use it, and you can continue ignoring it.

diek 3y ago

> I die a little bit each time I try to understand what changes were related to a line when tracking down a bug A change/feature/bug is a branch, which is squashed into a commit on your main branch, right? So your main branch should be a linear history of changes, one change per commit. How does that impact the ability to git blame?

joshribakoff 3y ago
Because sometimes a PR touches more code than a single commit, and you lose the more granular context surrounding the more granular changes. You can always ask git to make the log more coarse, but once you “destroy” the granular history it is for all intents and purposes gone.
js2 3y ago
Because unless it's the most trivial of features, you'll break it up into smaller commits which each explain what they are doing and make reviewing the change easier. As a simple example, I recently needed to update a json document that was a list of objects. I needed to add a new key/value to each object. The document had been hand edited over the years and had never been auto-formatted. My PR ended up being three commits: 1. Reformat the document with jq. Commit title explains it's a simple reformat of the document and that the next commit will add `.git-blame-ignore-revs` so that the history of the document isn't lost in `git blame` view. 2. Add `.git-blame-ignore-revs` with the commit ID of (1). 3. Finally, add the new key/value to each object. The PR then explains that a new key/value has been added, mentions that the document was reformatted through `jq` as part of the work, a recommends that the reviewer step through the commits to ignore the mechanical change made by (1). A followup PR added a pre-commit CI step to keep the document properly linted in the future.
- diek 3y ago
  In general I agree with you, there are absolutely times where you want to retain commit history on a particular branch (although I try to keep the source tree from knowing about things like commit IDs). I would argue that those are by far the minority of PRs that I see. As I mentioned in another comment, _most_ PRs that I see have a ton of intermediary commits that are only useful for that branch/PR/review process (fixing tests, whitespace, etc). Generally the advice I give teams is, "squash by default" and then figure out where the exceptions to that rule are. That's mainly because, in my opinion, the downsides of a noisy commit graph filled with "addressing review comments" (or whatever) commits are a much bigger/frequent issue than the benefits you talk about. It really depends on the team.

WirelessGigabit 3y ago

Me too. I care a LOT about provenance. And squash merge completely breaks that. When my branch is up to date with `main` I can build an artifact, fast forward merge that branch into `main` and RETAIN the artifact, and merely update its tags to mark it as `merged` in. With a squash I lose that information. Now, GitHub does not allow me to do a fast-forward merge but I can still trace the 2 commits that are the parent of the resultant merge, and find the artifact based on that, and retag.

Lio 3y ago

Git blame confuses people even without squash merges. I've seen people forget to go back more than one commit and then blame the person who last indented a file instead of going back to the commit that actually wrote the code many times.

erik_seaberg 3y ago
We shouldn’t tamper with code we don’t actually need to fix, it’s not a good use of time and it makes history less useful. Just because it doesn’t look like I wrote it doesn’t make it wrong.
- Lio 3y ago
  I’m thinking of situations where the surrounding structure of the code has been changed to correct a problem. That’s done by an automated tool. Correction of indentation is just a byproduct. I don’t consider that “tampering”.
  - deleted 3y ago
    [deleted]
  - WorldMaker 3y ago
    You can deal with that through tooling. In a lot of my work I call those types of automated tool commits "wrench" commits personally and even have a simple shell script to help automate committing them. In my case I prefix the command line with a wrench emoji. At that point it's very obvious in git blame that if a line starts with a wrench it was last touched by an automated tool of some sort. You can also very easily at that point grep your git log for wrenches to dump commit hashes into a git-ignore-revs file and automate that part too so that those commits don't even show up in git blame at all.
sodapopcan 3y ago
> I've seen people forget to go back more than one commit and then blame the person who last indented a file instead of going back to the commit that actually wrote the code many times. I default my `git blame` to `git blame -w` which ignores whitespace commits. Though knowing how to jump back commits should be required knowledge.