Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Git: Please Stop Squash Merging

lucasoshiro.github.io

Post reply on HN
Print view View original

11–20 of 32 posts

Re: Git: Please Stop Squash Merging

#11

Post by fargle »

yawaramin wrote: post #9

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…

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.

Re: Git: Please Stop Squash Merging

#12

Post by MartijnHols »

tail_exchange wrote: post #2

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…

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.

Re: Git: Please Stop Squash Merging

#13

Post by munksbeer »

MartijnHols wrote: post #12
tail_exchange wrote: post #2

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…

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.

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.

Re: Git: Please Stop Squash Merging

#14

Post by MartijnHols »

munksbeer wrote: post #13
MartijnHols wrote: post #12

Earlier quoted context omitted.

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.

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 t…

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?

Re: Git: Please Stop Squash Merging

#15

Post by tail_exchange »

MartijnHols wrote: post #12
tail_exchange wrote: post #2

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…

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.

If you use short-lived branches and every PR is one indivisible unit that represents a change, then it will always end up as a single commit anyways. When this is the case, there's no point enforcing a high commig quality - just squashing them will produce the same end-result.

Re: Git: Please Stop Squash Merging

#16

Post by lucasoshiro »

DreaminDani wrote: post #4
lucasoshiro wrote: post #3

Earlier quoted context omitted.

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 branch…

> 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.

> 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?

Re: Git: Please Stop Squash Merging

#17

Post by matheust »

munksbeer wrote: post #13
MartijnHols wrote: post #12

Earlier quoted context omitted.

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.

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 t…

> 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

I understand the workflow you've described here. I think the key word is "trivial". Otherwise, when I squash together many "large" patches, the end result will be a huge commit with possibly unrelated changes, and that can be really bad for code archeology. When the future me or someone else wants to revisit the history and try to understand why a change was made, it may be buried down a long list of changes in a single commit, and the commit message won't be able to proper explain the reasoning behind it, IMHO.

Something that I also quite like about having individual patches in a PR/MR, is being able to review them individually too. Makes it easier for me, as a reviewer, to be able to understand the motivations behind each set of changes. But then again, that may only apply for larger patchesets/PRs/MRs...

Re: Git: Please Stop Squash Merging

#18

Post by lucasoshiro »

MartijnHols wrote: post #12
tail_exchange wrote: post #2

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…

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.

> The poor commit quality is your issue here, and squashing is merely a way to avoid addressing it.

Just like I said: it's like driving reverse because you don't want to learn how to shift gears!

> That only makes squashing a good tool to use to make a big mess a slightly smaller mess.

You're right, in cases that the repository is so cluttered this may be a palliative solution to avoid things to get worse. Squash exists, so it is a tool that can be used if you know what you're doing. But it being useful for rescuing in some situations doesn't mean that can be used in all situations. It's like taking pills instead of vaccines.

There are other tools in this category, such as git push -f: it is a useful tool that you may use when it's necessary if know what you're doing. This doesn't mean that it should be used as first option. Same for squash

Re: Git: Please Stop Squash Merging

#19

Post by lucasoshiro »

strawhatguy wrote: post #6

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 re…

> the comments to the commits were several hundred MiB

I did't even thought about it. In fact, those are hundreds of lines that no one would read, as the commits are lost and you can't do anything with them...

Re: Git: Please Stop Squash Merging

#20

Post by lucasoshiro »

yawaramin wrote: post #8
strawhatguy wrote: post #6

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 re…

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).

> 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-r...

Funny that they don't mention the name collision as reason for renaming it...

Post reply on HN