5 ms·

Git: Please Stop Squash Merging

20 points by lucasoshiro 2y ago

tail_exchange 2y ago
My experience has been the opposite. Every organization I worked at used Trunk based development (feature branch->master). Because branches are short lived and usually small, they are the atomic representation of a change, and not the commits in the pull request. Pull requests usually have lots of "linting", "fixing test", "fixing typo" commits that I absolutely do not want in my master branch. I understand that not all organizations work like this, but that is my point: there is nuance. Blanket statements like these are not productive. The author seems frustrated that people still use squash merge, and just assumes that it's because people just don't understand how git works.
- lucasoshiro 2y ago
  Hi! Thanks for the comment! > My experience has been the opposite. Every organization I worked at used Trunk based development (feature branch->master). Each repository has its own necessities and you need to adapt the workflow to them. Sometimes for small repos what's work the best is commits on main, some people like the so-called "git flow", and Linux have different maintainers for each subsystem. > Because branches are short lived and usually small, they are the atomic representation of a change, and not the commits in the pull request. If they are small enough they can be a single commit. No problem in doing that, and the result will be similar to squashing. If they need more than one commit, so they are not exactly "small" > Pull requests usually have lots of "linting", "fixing test", "fixing typo" commits that I absolutely do not want in my master branch. I agree that if someone commits something and then commits a "lint", "fix typo", this could be solved in commit correctly once. But this is not always possible, so one can use the rewriting history tools that were mentioned. But I return the question: what are the downsides of having those commits? > Blanket statements like these are not productive. What, exactly, was blank? I tried to justify everything that I could based on the how Git works and debunk statements like "squash makes the history cleaner" (which, in fact, are blank). > just assumes that it's because people just don't understand how git works. So, I make the same question of the text: after reading and knowing what squash merges really are, what are the good reasons to still use them?
  - DreaminDani 2y ago
    > what are the downsides of having those commits? In my experience, having to rebase on top of a branch with a lot of "lint" "quick fix" "let's try this" "oops, how about this" can be draining and sometimes lead to accidental code deletion. I like squash merges because I know it was the author's intended change that I'm merging or rebasing onto.
    - kstenerud 2y ago
      Yup, agreed. I've worked in orgs that use long lived branches and short lived branches... And short lived branches are by far less stressful to deal with if you can do it, precisely because of merge conflicts. Reality is never going to match your desire. Things are going to get messy, and so you end up with squashable commits among your commits that shouldn't be squashed. The branch you're merging should match your intent for atomic changes to the code that can be easily reasoned about when rebasing, merging, and bisecting. You write to history once, but you read from it many times. Therefore, optimize for reads.
    - lucasoshiro 2y ago
      > In my experience, having to rebase on top of a branch with a lot of "lint" "quick fix" "let's try this" "oops, how about this" can be draining and sometimes lead to accidental code deletion. " But this is not always possible, so one can use the rewriting history tools that were mentioned." > I like squash merges because I know it was the author's intended change that I'm merging or rebasing onto. Can you elaborate, please?
- MartijnHols 2y ago
  The poor commit quality is your issue here, and squashing is merely a way to avoid addressing it. That only makes squashing a good tool to use to make a big mess a slightly smaller mess.
  - munksbeer 2y ago
    Thank you, but no. I enjoy working on a branch and committing small changes at a time, sometimes trivial, sometimes not even compiling, sometimes formatting or whatever the hell I feel like. When it comes time to review, I squash all those trivial changes manually (rebase -i) and present a clean branch for review. We could try harder to enforce stricter policies on devs to ensure they're much more disciplined about their initial commits, but what we do works for us. It allows faster iteration and then we enforce squash and merge once ready to go to master. I don't understand why this bothers some people so much that they spend time ranting "you're using git wrong". If it was wrong, we'd be noticing and change our workflow. But it doesn't feel wrong, so we'll continue to use it.
    - MartijnHols 2y ago
      Interactive rebasing away fixups in your branch is not squash merging. I agree that fixing a typo in a variable or a small linting issue in a previous (recent or not-pushed) commit in your branch is usually good. A carefully crafted commit history is all anyone can ask for. But then squashing away that history once the PR is merged would be a waste of a good history and the effort you put into it. If you end up squashing your history away anyway, why bother cleaning it up in an interactive rebase?
      - munksbeer 2y ago
        I can't follow what you're saying. Once people start reviewing the PR, we typically do not manually squash (rebase -i) because it is helpful to see how the flow of the review went. Following this, we sometimes end up with 10 commits on a PR that are just addressing various comments, nitpicky or not. Then once approved, we click the "squash and merge" button, and github squashes all that useless noise into a single commit on master for us. We don't have any complaints about this workflow.
strawhatguy 2y ago
My bad experience with squash merging was with Stash at my last job. Some developers there thought the "clean history" was so worth it they made squash-merging the default merge strategy. The result: the git repository (at least in Stash) slowed to a crawl, because the comments to the commits were several hundred MiB. It was nuts. Never seen a git hosting site go that slow before. Actually had to replace the whole repo with another, more or less wiping history (kept the old slow repo just in case, of course), and disabling that damn squash-merge button.
- yawaramin 2y ago
  Sounds like the problem was not squash merging, but the slow and buggy implementation of Stash (hilariously bad naming for a git-related product btw).
  - lucasoshiro 2y ago
    > hilariously bad naming for a git-related product btw) I only understood what "Stash" is in this situation after your comment. Looks like it is an Atlassian product that was renamed: https://confluence.atlassian.com/bitbucketserver/bitbucket-rebrand-faq-779298912.html https://confluence.atlassian.com/bitbucketserver/bitbucket-r... Funny that they don't mention the name collision as reason for renaming it...
    - strawhatguy 2y ago
      I think Atlassian built their own, and bought the other. And now it's bitbucket. Maybe nothing left of Stash anymore. Maybe that's a good thing.
  - strawhatguy 2y ago
    Yeah, it was Atlassian's git thing, or one they bought, and combined it with bitbucket or something. I'll say it was with the configuration and the buggy implementation of squash merging button with Stash. After replacing the repo, people were amazed with how fast it was. Also apparently I became notorious as the committer of the whole repository, since first commit was mine!
kevmo314 2y ago
This is such an engineer's response to a usability problem. I've worked with quite a few organizations and many of them use squash merges. Commits are often made because of a failed lint check needs a new formatting commit, addressing some comments to tweak some minor details, or even basic documentation changes. The pull request on GitHub ends up being the source of truth for the change, not the git log, and squash merges do a great job at keeping a single pointer to the parent pull request. Ignoring these nuances and how git interacts with other tools amounts to telling the user they're holding it wrong. Clearly, squash merging does something useful but the author is totally not interested and declares that squash merging is good for one exceptionally specific use case, the only true clean commit history is not using git at all, and the problem exists between the keyboard and the computer.
yawaramin 2y ago
This post is full of weird non sequiturs and logical fallacies. So what if git doesn't have a single command to 'squash-merge'? It doesn't natively support a 'pull request' workflow either. Code forges build convenience functionality on top of git. If I didn't have the 'squash merge' feature I'd just use an interactive rebase to fixup/squash the branch's commits before doing a fast-forward-only merge, because I don't want a git history littered with meaningless noise. And the 'fewer commits are better so that must mean no commits is best' argument is such an obvious slippery slope fallacy, come on man. Do you seriously think this kind of argument will fly in real life? You are just going to annoy the crap out of everyone. We don't squash and rebase because we want fewer commits, we do it because we want fewer meaningless noise commits. Do you want to preserve all your editor keystrokes, like backspaces, moving code around, copy-pasting, etc., before committing? If you don't, then you obviously don't want to track any history, right? Of course not, that's silly. That's what your argument is like. I don't want my repo history littered with meaningless noise like these merge commit messages: * Merge pull request #1234 from bob/implement-foo * Implement foo (pull request #1234) The only thing that matters here is the log of the actual work, not the incidental artifact that git merged something. I also don't want a commit history that looks like London's tube system map. Please, keep the history linear so that myself and future team members can quickly focus on the parts that interest them, after just a glance, and don't have to spend time decoding the tangled mess of interlocking branch histories. Much appreciated!
- fargle 2y ago
  upvoting! "I also don't want a commit history that looks like London's tube system map" i don't care how you do it. squashing? git reset followed by sensible commits? intense ninja-like git commit hygiene? or my favorite, liberal amounts of git rebase -i? the only things i care about are these: 1. your PR contains a series of atomic commits 2. each commit makes a single logical change. 3. that each logical change is adequately described by the corresponding commit comment 4. extra credit if the series of changes is consistent to build and work at each step i don't want to see "ONE" commit per-feature unless it's trivial. i do want to see the development process, but in a "publish"-able form i don't want to see "WIP", "almost working", "fixed broken", "redesigned ...", etc. sausage being made. i especially DON'T want to see patches to files be changed and reverted multiple times as mistakes are made and fixed. the final PR should be you publishing a nice finished series of patches to accomplish a new feature. then! we simply rebase that nice simple series on top of master and merge it with an optional merge commit. this gets you a nice linear history while avoiding the information loss with simply squashing everything in a PR/feature into a single commit.
rednafi 2y ago
No.

20 points by lucasoshiro 2y ago

My experience has been the opposite. Every organization I worked at used Trunk based development (feature branch->master). Because branches are short lived and usually small, they are the atomic representation of a change, and not the commits in the pull request. Pull requests usually have lots of "linting", "fixing test", "fixing typo" commits that I absolutely do not want in my master branch. I understand that not all organizations work like this, but that is my point: there is nuance. Blanket statements like these are not productive. The author seems frustrated that people still use squash merge, and just assumes that it's because people just don't understand how git works.

lucasoshiro 2y ago
Hi! Thanks for the comment! > My experience has been the opposite. Every organization I worked at used Trunk based development (feature branch->master). Each repository has its own necessities and you need to adapt the workflow to them. Sometimes for small repos what's work the best is commits on main, some people like the so-called "git flow", and Linux have different maintainers for each subsystem. > Because branches are short lived and usually small, they are the atomic representation of a change, and not the commits in the pull request. If they are small enough they can be a single commit. No problem in doing that, and the result will be similar to squashing. If they need more than one commit, so they are not exactly "small" > Pull requests usually have lots of "linting", "fixing test", "fixing typo" commits that I absolutely do not want in my master branch. I agree that if someone commits something and then commits a "lint", "fix typo", this could be solved in commit correctly once. But this is not always possible, so one can use the rewriting history tools that were mentioned. But I return the question: what are the downsides of having those commits? > Blanket statements like these are not productive. What, exactly, was blank? I tried to justify everything that I could based on the how Git works and debunk statements like "squash makes the history cleaner" (which, in fact, are blank). > just assumes that it's because people just don't understand how git works. So, I make the same question of the text: after reading and knowing what squash merges really are, what are the good reasons to still use them?
- DreaminDani 2y ago
  > what are the downsides of having those commits? In my experience, having to rebase on top of a branch with a lot of "lint" "quick fix" "let's try this" "oops, how about this" can be draining and sometimes lead to accidental code deletion. I like squash merges because I know it was the author's intended change that I'm merging or rebasing onto.
  - kstenerud 2y ago
    Yup, agreed. I've worked in orgs that use long lived branches and short lived branches... And short lived branches are by far less stressful to deal with if you can do it, precisely because of merge conflicts. Reality is never going to match your desire. Things are going to get messy, and so you end up with squashable commits among your commits that shouldn't be squashed. The branch you're merging should match your intent for atomic changes to the code that can be easily reasoned about when rebasing, merging, and bisecting. You write to history once, but you read from it many times. Therefore, optimize for reads.
  - lucasoshiro 2y ago
    > In my experience, having to rebase on top of a branch with a lot of "lint" "quick fix" "let's try this" "oops, how about this" can be draining and sometimes lead to accidental code deletion. " But this is not always possible, so one can use the rewriting history tools that were mentioned." > I like squash merges because I know it was the author's intended change that I'm merging or rebasing onto. Can you elaborate, please?
MartijnHols 2y ago
The poor commit quality is your issue here, and squashing is merely a way to avoid addressing it. That only makes squashing a good tool to use to make a big mess a slightly smaller mess.
- munksbeer 2y ago
  Thank you, but no. I enjoy working on a branch and committing small changes at a time, sometimes trivial, sometimes not even compiling, sometimes formatting or whatever the hell I feel like. When it comes time to review, I squash all those trivial changes manually (rebase -i) and present a clean branch for review. We could try harder to enforce stricter policies on devs to ensure they're much more disciplined about their initial commits, but what we do works for us. It allows faster iteration and then we enforce squash and merge once ready to go to master. I don't understand why this bothers some people so much that they spend time ranting "you're using git wrong". If it was wrong, we'd be noticing and change our workflow. But it doesn't feel wrong, so we'll continue to use it.
  - MartijnHols 2y ago
    Interactive rebasing away fixups in your branch is not squash merging. I agree that fixing a typo in a variable or a small linting issue in a previous (recent or not-pushed) commit in your branch is usually good. A carefully crafted commit history is all anyone can ask for. But then squashing away that history once the PR is merged would be a waste of a good history and the effort you put into it. If you end up squashing your history away anyway, why bother cleaning it up in an interactive rebase?
    - munksbeer 2y ago
      I can't follow what you're saying. Once people start reviewing the PR, we typically do not manually squash (rebase -i) because it is helpful to see how the flow of the review went. Following this, we sometimes end up with 10 commits on a PR that are just addressing various comments, nitpicky or not. Then once approved, we click the "squash and merge" button, and github squashes all that useless noise into a single commit on master for us. We don't have any complaints about this workflow.

strawhatguy 2y ago

My bad experience with squash merging was with Stash at my last job. Some developers there thought the "clean history" was so worth it they made squash-merging the default merge strategy. The result: the git repository (at least in Stash) slowed to a crawl, because the comments to the commits were several hundred MiB. It was nuts. Never seen a git hosting site go that slow before. Actually had to replace the whole repo with another, more or less wiping history (kept the old slow repo just in case, of course), and disabling that damn squash-merge button.

yawaramin 2y ago
Sounds like the problem was not squash merging, but the slow and buggy implementation of Stash (hilariously bad naming for a git-related product btw).
- lucasoshiro 2y ago
  > hilariously bad naming for a git-related product btw) I only understood what "Stash" is in this situation after your comment. Looks like it is an Atlassian product that was renamed: https://confluence.atlassian.com/bitbucketserver/bitbucket-rebrand-faq-779298912.html https://confluence.atlassian.com/bitbucketserver/bitbucket-r... Funny that they don't mention the name collision as reason for renaming it...
  - strawhatguy 2y ago
    I think Atlassian built their own, and bought the other. And now it's bitbucket. Maybe nothing left of Stash anymore. Maybe that's a good thing.
- strawhatguy 2y ago
  Yeah, it was Atlassian's git thing, or one they bought, and combined it with bitbucket or something. I'll say it was with the configuration and the buggy implementation of squash merging button with Stash. After replacing the repo, people were amazed with how fast it was. Also apparently I became notorious as the committer of the whole repository, since first commit was mine!

kevmo314 2y ago

This is such an engineer's response to a usability problem. I've worked with quite a few organizations and many of them use squash merges. Commits are often made because of a failed lint check needs a new formatting commit, addressing some comments to tweak some minor details, or even basic documentation changes. The pull request on GitHub ends up being the source of truth for the change, not the git log, and squash merges do a great job at keeping a single pointer to the parent pull request. Ignoring these nuances and how git interacts with other tools amounts to telling the user they're holding it wrong. Clearly, squash merging does something useful but the author is totally not interested and declares that squash merging is good for one exceptionally specific use case, the only true clean commit history is not using git at all, and the problem exists between the keyboard and the computer.

yawaramin 2y ago

This post is full of weird non sequiturs and logical fallacies. So what if git doesn't have a single command to 'squash-merge'? It doesn't natively support a 'pull request' workflow either. Code forges build convenience functionality on top of git. If I didn't have the 'squash merge' feature I'd just use an interactive rebase to fixup/squash the branch's commits before doing a fast-forward-only merge, because I don't want a git history littered with meaningless noise. And the 'fewer commits are better so that must mean no commits is best' argument is such an obvious slippery slope fallacy, come on man. Do you seriously think this kind of argument will fly in real life? You are just going to annoy the crap out of everyone. We don't squash and rebase because we want fewer commits, we do it because we want fewer meaningless noise commits. Do you want to preserve all your editor keystrokes, like backspaces, moving code around, copy-pasting, etc., before committing? If you don't, then you obviously don't want to track any history, right? Of course not, that's silly. That's what your argument is like. I don't want my repo history littered with meaningless noise like these merge commit messages: * Merge pull request #1234 from bob/implement-foo * Implement foo (pull request #1234) The only thing that matters here is the log of the actual work, not the incidental artifact that git merged something. I also don't want a commit history that looks like London's tube system map. Please, keep the history linear so that myself and future team members can quickly focus on the parts that interest them, after just a glance, and don't have to spend time decoding the tangled mess of interlocking branch histories. Much appreciated!

fargle 2y ago
upvoting! "I also don't want a commit history that looks like London's tube system map" i don't care how you do it. squashing? git reset followed by sensible commits? intense ninja-like git commit hygiene? or my favorite, liberal amounts of git rebase -i? the only things i care about are these: 1. your PR contains a series of atomic commits 2. each commit makes a single logical change. 3. that each logical change is adequately described by the corresponding commit comment 4. extra credit if the series of changes is consistent to build and work at each step i don't want to see "ONE" commit per-feature unless it's trivial. i do want to see the development process, but in a "publish"-able form i don't want to see "WIP", "almost working", "fixed broken", "redesigned ...", etc. sausage being made. i especially DON'T want to see patches to files be changed and reverted multiple times as mistakes are made and fixed. the final PR should be you publishing a nice finished series of patches to accomplish a new feature. then! we simply rebase that nice simple series on top of master and merge it with an optional merge commit. this gets you a nice linear history while avoiding the information loss with simply squashing everything in a PR/feature into a single commit.

rednafi 2y ago

No.