Two Years of Squash Merge (2019) • Hacker News Board

Re: Two Years of Squash Merge (2019)

#51

Post by pizza234 » Fri, Apr 30, 2021, 7:52 PM UTC

ginja wrote: post #10

gouggoug wrote: post #2

I will always fight tooth and nail against squash merge. Squash merge has the major disadvantage of getting rid of valuable meaningful git history. Squash merge is not the proper solution for keeping your git history clean, it is a hack using the side effect of squash. Keeping your git history clean is a matter of policy, best-practices and education: Developers should be required to submit _clean_ PRs, that is, PR's…

I do agree that PRs should have a clean history. However: - Services like GitHub allow you to restore the original branch, so you never actually lose history. So I don't see any major drawbacks of squashing on merge. - If your PRs are several thousand lines long, they probably should've been broken up into multiple PRs (your reviewer will appreciate it)

> - Services like GitHub allow you to restore the original branch, so you never actually lose history. So I don't see any major drawbacks of squashing on merge.

There's definitely a drawback, and it's bisecting, which is actually a big deal.

Bisecting allows in a semi-automated (depending on the issue) bisecting what otherwise can be a large diff.

But of course, it requires a disciplined history - otherwise, bisecting just won't work.

Re: Two Years of Squash Merge (2019)

#52

Post by pizza234 » Fri, Apr 30, 2021, 7:54 PM UTC

thom wrote: post #48

calpaterson wrote: post #5

Earlier quoted context omitted.

> Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. A complete and utter waste of time. You spend more time messing about with rebase than solving problems. When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seeme…

Do people here have examples of some bugs for which they had to resort to VCS history to find the cause? I'm struggling to picture a single bug in my whole career where this would have been quicker than just following the logic of the code. If there's information in commit messages that isn't evident in the code itself, that seems a terrible way to live.

As wrote in another comment, bisecting (which is for me a significant tool) relies on history (specifically, a granular one). However, it also must a disciplined history.

Re: Two Years of Squash Merge (2019)

#53

Post by gregmac » Fri, Apr 30, 2021, 7:56 PM UTC

calpaterson wrote: post #5

gouggoug wrote: post #2

I will always fight tooth and nail against squash merge. Squash merge has the major disadvantage of getting rid of valuable meaningful git history. Squash merge is not the proper solution for keeping your git history clean, it is a hack using the side effect of squash. Keeping your git history clean is a matter of policy, best-practices and education: Developers should be required to submit _clean_ PRs, that is, PR's…

> Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. A complete and utter waste of time. You spend more time messing about with rebase than solving problems. When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seeme…

> A complete and utter waste of time. You spend more time messing about with rebase than solving problems.

Unless you're using a garbage client (eg, the git CLI) a rebase to get rid of the "typo" "oops" type commits (when you forgot something) takes I'd say 10-20 seconds.

Most commonly I do this when I have several changes on the go at once and forget to commit a fixed unit test, new import, or something like that. I usually realize it right away, so my commit will usually be "add test" or "amend 2 commits ago" or something to remind myself which one it goes to (and it sticks out, because my other commits are usually in the format "PROJ-1234: Add new --host command-line option").

> When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seemed to do you wouldn't be there.

I presume this is doing a bisect or something to narrow down where a bug was introduced?

My experience comes from the opposite side. Usually I identify the line of code causing a bug, it makes me go "wtf, what is this even supposed to be doing?" so I do a git blame. Hopefully the commit message is useful and ideally leads me back to the original bug/ticket, so I can figure out what the original intent/fix was, rather than inadvertently breaking something (or re-causing a different bug it fixed). I'll also note that in code that is well-commented and well-tested, this step is rarely necessary.

As an example, say I come across something like this:

    if (port > 443) ignoreErrors = true;

Clearly, this was added to solve some some problem, but even without any context it's pretty obviously a bad solution to whatever that problem is. Git blame lets me go back to see what the original reason/bug was, evaluate if it's still relevant and then either fix it properly or safely remove this line.

Re: Two Years of Squash Merge (2019)

#54

Post by detaro » Fri, Apr 30, 2021, 8:06 PM UTC

thom wrote: post #48

calpaterson wrote: post #5

Earlier quoted context omitted.

> Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. A complete and utter waste of time. You spend more time messing about with rebase than solving problems. When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seeme…

Do people here have examples of some bugs for which they had to resort to VCS history to find the cause? I'm struggling to picture a single bug in my whole career where this would have been quicker than just following the logic of the code. If there's information in commit messages that isn't evident in the code itself, that seems a terrible way to live.

As parent said, history is not just commit messages.

Trivial example from yesterday: "this worked last week... what did they even touch inbetween?", look at the most relevant commit diff, context makes it obvious someone just accidentally deleted a line too many while replacing a code block.

For more complex stuff, it's helpful for "what were they trying to do with this line", "what's the requirements document they worked off when they wrote this", "which other versions are likely to have this bug too", "Is this new and I can go ask whoever wrote this about it or is it years old", ...

Re: Two Years of Squash Merge (2019)

#55

Post by lifeisstillgood » Fri, Apr 30, 2021, 8:13 PM UTC

gouggoug wrote: post #2

I will always fight tooth and nail against squash merge. Squash merge has the major disadvantage of getting rid of valuable meaningful git history. Squash merge is not the proper solution for keeping your git history clean, it is a hack using the side effect of squash. Keeping your git history clean is a matter of policy, best-practices and education: Developers should be required to submit _clean_ PRs, that is, PR's…

I think you are using got commit log as a form of documentation.

I half-agree. I don't agree devs should spend (much) time cleaning up their logs as opposed to actually writing docs (inline) that help the next person

I do agree that there is rarely a good (even half good) history of decisions. This is never the Jira / tickets kept outside of the system.

But it is also no good abusing the commit log as a form of ... journal. (I know that sounds silly but )

I think an actual journal / blog kept by the dev lead will be much more useful

Re: Two Years of Squash Merge (2019)

#56

Post by rubyist5eva » Fri, Apr 30, 2021, 8:19 PM UTC

waffletower wrote: post #46

rubyist5eva wrote: post #12

Earlier quoted context omitted.

In the article he says he keeps the original branch with all of the history around after they squash them with a reference to the original PR in the squashed merge commit message. So you can always just checkout the original branch and go digging in the full history.

Unwieldy to keep branches around even on moderate sized teams/projects. You don't need them to have a `squashed` view of history when needed. I am surprised that developers still cling to this outdated squashing regimen when pull request tools found on github, gitlab, bitbucket etc. already provide synthetic squash views derived from atomic commits by default.

How is it unwieldy? They don't even show up locally when you `git branch` unless you've checked them out. Otherwise they just sit there doing nothing, there is literally no maintenance required on them once they've been squashed into mainline. Most central repositories end up having hundreds (or even thousands) of "finished" branches that nobody ever looks at anyway.

Re: Two Years of Squash Merge (2019)

#57

Post by waffletower » Fri, Apr 30, 2021, 8:26 PM UTC

rubyist5eva wrote: post #56

waffletower wrote: post #46

Earlier quoted context omitted.

Unwieldy to keep branches around even on moderate sized teams/projects. You don't need them to have a `squashed` view of history when needed. I am surprised that developers still cling to this outdated squashing regimen when pull request tools found on github, gitlab, bitbucket etc. already provide synthetic squash views derived from atomic commits by default.

How is it unwieldy? They don't even show up locally when you `git branch` unless you've checked them out. Otherwise they just sit there doing nothing, there is literally no maintenance required on them once they've been squashed into mainline. Most central repositories end up having hundreds (or even thousands) of "finished" branches that nobody ever looks at anyway.

Depends on configuration if you wind up having that many. I use `git branch -v` often to figure out what branch a colleague is working on. I guess I could apply my same logic and use filtering tools here as well. But I don't see the benefit of squashing the merge commits when git CLI, github etc. can give you the same view without the gratuitous data loss.

Re: Two Years of Squash Merge (2019)

#58

Post by thom » Fri, Apr 30, 2021, 8:52 PM UTC

pizza234 wrote: post #52

thom wrote: post #48

Earlier quoted context omitted.

Do people here have examples of some bugs for which they had to resort to VCS history to find the cause? I'm struggling to picture a single bug in my whole career where this would have been quicker than just following the logic of the code. If there's information in commit messages that isn't evident in the code itself, that seems a terrible way to live.

As wrote in another comment, bisecting (which is for me a significant tool) relies on history (specifically, a granular one). However, it also must a disciplined history.

What's an example of a bug you've had to bisect for recently? Forgive me, it just seems like such a last resort thing.

Re: Two Years of Squash Merge (2019)

#59

Post by FalconSensei » Fri, Apr 30, 2021, 8:57 PM UTC

gregmac wrote: post #53

calpaterson wrote: post #5

Earlier quoted context omitted.

> Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. A complete and utter waste of time. You spend more time messing about with rebase than solving problems. When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seeme…

> A complete and utter waste of time. You spend more time messing about with rebase than solving problems. Unless you're using a garbage client (eg, the git CLI) a rebase to get rid of the "typo" "oops" type commits (when you forgot something) takes I'd say 10-20 seconds. Most commonly I do this when I have several changes on the go at once and forget to commit a fixed unit test, new import, or something like that. I…

> My experience comes from the opposite side. Usually I identify the line of code causing a bug, it makes me go "wtf, what is this even supposed to be doing?" so I do a git blame.

If the commit message is something like: PROJECT-TICKETNUMBER Description, you can you know... see the ticket/issue along with full details and discussions?

Re: Two Years of Squash Merge (2019)

#60

Post by Pxtl » Fri, Apr 30, 2021, 9:01 PM UTC

Squash was the thing that convinced me that the emperor has no clothes. Realizing that I was going to either have to train every junior, every four-month community-college student brought in on co-op to modify their history in an awful UI with tons of gotchas, or I would have to accept the downsides of squash?

It's so stupid.

Git desperately needs a layer above the commit that groups related commits together into a semantically commit-like object that you can show in history and jump to its HEAD and cherry-pick. Because the squash is a dumb hack, and meticulously editing your history is not productive work.

I want squash. But I want squash without all the boneheaded implementation-detail downsides of squash. I want squash where I can put up a PR and keep working and then not have to deal with the cherry-pick pain if I want to build off that work after it merges. I want squash where I can leave the branch up after the merge and still see that it's behind the main branch.

But git's simple "everything is a commit" model makes that impossible.