Live data from Hacker News

Two Years of Squash Merge (2019)

blog.dnsimple.com

31–40 of 194 posts

Re: Two Years of Squash Merge (2019)

#31

The older I get the more I find these discussion as counterproductive as figuring out where to put the bike shed. I worked in teams that did squash merge and in teams that didn't. And in teams where some did and some didn't, on the same repo. In the grand scheme of things it didn't matter, except for hardliners who had nothing better to talk about.

I agree with your point, but, tangentially, I do find it amusing that "where to put the bike shed" is actually _more_ impactful than what I thought "bike-shedding" referred to (arguing about what _colour_ to paint the bike shed for a nuclear reactor). At least the location of the bike shed actually has some (small) effect on people's commute!

Re: Two Years of Squash Merge (2019)

#32
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…

Hey, so this POV comes up a lot, and I have to say that I think it mistakes how git commits should work, but I’ll add that you can sort of have both.

First, in a production branch, git commits should be thought of as functions. Like “Apply commit X, get feature Y, unapply it and you get the reverse”. So the problem with preserving full git history in master is that it breaks that invariant. You have to sort of do like a range of commits, but then that doesn’t really always work because often times other commits can be interleaved into yours.

To understand how I mean, just look at Linux or open source software projects, where the technical experts are more or less gatekeepers and aren’t accountable to any other influence. You’ll find that commits work this way, and their history is squashed. (Maintainers will also force you to rebase before merging and basically put all the work on you to get the PR in ship shape, which is a lot different from a corporate environment)

Ok, but then to your point about preserving valuable, more granular commits, well, the solution is you just leave that remote branch up, either in your fork or elsewhere (but probably in your fork). These branches should have formal names identifying a ticket (at work) or an RFC or whatever, so it should be easy for people to discover what happened. They can see this remote issued a PR to this remote to implement RFC-123 or whatever, and they can go to that remote and see the granular commits if you preserved.

Sorry, this is like a never ending debate and there are very strong opinions, but I sincerely think that in this situation there is actually a right answer. People who already know how to do this or are super familiar with open source, maintainers or team leads or whatever, I think don’t see the point in fighting over it, since they can just enforce whatever they want in their gatekeeper role. However, the truth is that there’s sort of a fundamental misunderstanding that the majority of the population has around git and I think it’s better to engage and explain when you get the chance.

Re: Two Years of Squash Merge (2019)

#33
It would be nice if GitHub allowed comments on commit messages and not only the changes so that they could be discussed for the benefit of learning and improvement.

Squash merge commit messages are currently not reviewable at all since they are not entered until just before the merge.

I'd prefer for nothing at all to be able to enter the main branch without review, if not for anything else to protect myself against my own mistakes.

Guess I should give Gerrit a shot.

Re: Two Years of Squash Merge (2019)

#34
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…

> When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug.

So, the only way that I can think that having the squash commit broken into individual commits would help to find the one broken line is because it would enable git bisect to find the failing commit.

However, that implies that each individual commit inside that feature branch worked on its own. If that is the case, a better suggestion would be to break up the giant feature branch into smaller sub features that can be merged as they are completed

Re: Two Years of Squash Merge (2019)

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

I'm also one of those "micro committers".

Because of this attitude, many hundreds pedantic commit rebases after, I'm now a better programmer (and I also spend next to no time with that type of rebases).

This is because I have now a much higher capacity (and speed) of breaking down problems into smaller, self-contained, steps.

There is a common understanding that VCs are just storages. VCs actually map the mental model of the developer. Having a precise, clear, granular VC history has a bidirectional relationship with being a precise, granular, clear developer.

Re: Two Years of Squash Merge (2019)

#36

Earlier quoted context omitted.

I would agree with your points if not for the fact that "clean up commits" or "typo fix" is a necessary result of PR's. Your teammate will request changes in your code, and the only way to cleanly communicate "yes I made that change, and ONLY that change" is through these clean-up commits. Otherwise, if you amend/force-push or open an entirely new PR, 99% of the diff are things that your team has already seen and rev…

> Otherwise, if you amend/force-push or open an entirely new PR, 99% of the diff are things that your team has already seen and reviewed. gerrit has solved this issue for years by showing the diffs between each successive revision of a patch. e.g. look here the files at different origin patchsets : https://codereview.qt-project.org/c/qt/qtwayland/+/321246/3....

It took me a while to get a workflow that actually works with Gerrit, and I occasionally think of trying to do some kind of autosquash so I can have successive commits locally. In practice I just use --amend all the time.

Careful use of setting upstream and of course pull=rebase makes keeping up with trunk manageable.

Re: Two Years of Squash Merge (2019)

#37
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…

> You spend more time messing about with rebase than solving problems

Rebase should be quick and straightforward. If it's not, you've got bigger problems.

By rebasing, you're taking responsibility for changing the codebase as it exists now, rather than how it used to be.

Re: Two Years of Squash Merge (2019)

#38
My old team’s git workflow required us to rebase our feature branches to develop before merging, and our branch could only contain a single commit. The commit needed a special formatting (short title, description, and story number) so it would get picked up by Jira scripts.

Although the history was super clean, the extra upkeep was a small annoyance. I felt like tearing my hair out when there were 10 other merge requests pending and never knew which would be the next to merge with develop. An auto-rebase feature (assuming no merge conflicts) would have saved me countless pointless minutes.

Re: Two Years of Squash Merge (2019)

#39
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…

I think I'm missing something here. How valuable is to have 20 commits of "fix this error" , "fix the fix of the error", "revert all fixes", "real fix".... etc? I'd argue a PR with many commits such as these, conveys little no no useful information, when the actual change is 1-3 LOC.

what about cleaning and filtering out useless commits, by soft reseting the branch and commiting just the actual changes to merge. Sure, it can take a lot of time, but it's done once, by you, after all you're supplying the changes. And if it takes a really long time, you're doing it wrong in the first place by submitting PRs with many LOC.

Have you tried rebasing branches with dozens of short useless commits? That's a real waste of time, and everyone needs to do this if you want to update your main branch. So you've now multiplied the amount of wasted time for everyone.

Re: Two Years of Squash Merge (2019)

#40
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 would agree with your points if not for the fact that "clean up commits" or "typo fix" is a necessary result of PR's. Your teammate will request changes in your code, and the only way to cleanly communicate "yes I made that change, and ONLY that change" is through these clean-up commits. Otherwise, if you amend/force-push or open an entirely new PR, 99% of the diff are things that your team has already seen and rev…

I don't mind this. I want my reviewers to look at the state of the new code - not the diff, and not the diff of the diff.

Address PR comments using the PR comments.

Post reply on HN