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

Two Years of Squash Merge (2019)

blog.dnsimple.com

Post reply on HN
Print view View original

161–170 of 194 posts

Re: Two Years of Squash Merge (2019)

#161

Post by ThrustVectoring »

tharkun__ wrote: post #90
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…

You have apparently never had the benefit of a properly run git repository. Our master has straight history. We always rebase to merge a PR and we have build scripts that check this as well. You will not be able to merge a PR that tries to get more than one commit onto master, if the commits touch more than one of the modules (we have a monorepo with lots of individual services). Until the PR has been approved we lea…

> if the commits touch more than one of the modules (we have a monorepo with lots of individual services)

That seems wrong to me - a big advantage to having a monorepo is that when you need to make a change to how two services work together, you can do so with a single atomic commit.

Re: Two Years of Squash Merge (2019)

#162

Post by ThrustVectoring »

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.

I've used version control history to verify that a mysterious bug was not caused by any changes in the codebase. Instead, we were set up to automatically incorporate minor version changes in dependencies, one of which improperly had a breaking change.

This class of bug is impossible to find by following the logic of the code because it's not in the code.

Re: Two Years of Squash Merge (2019)

#163

Post by david_allison »

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.

"had to" is a stretch, but as the first example from GitHub: https://github.com/ankidroid/Anki-Android/issues/7781

There's a lot of value in bisecting to find the exact commit, moving from "this is probably the commit that broke it" to "this is exactly the commit which broke it"

Typical workflow:

* branch off master

* Write a test which passes at some point in the past, and fails on HEAD

* rebase the test into the branch at the point in the past, so bisect works

* git bisect to pinpoint the bug

With a non-squashed and a history where each commit builds, `git bisect` points to the exact failing commit and makes the investigation trivial

An additional advantage is that you're not wasting time investigating: the unit test can be used as a regression test

Re: Two Years of Squash Merge (2019)

#164

Post by tharkun__ »

ThrustVectoring wrote: post #161
tharkun__ wrote: post #90

Earlier quoted context omitted.

You have apparently never had the benefit of a properly run git repository. Our master has straight history. We always rebase to merge a PR and we have build scripts that check this as well. You will not be able to merge a PR that tries to get more than one commit onto master, if the commits touch more than one of the modules (we have a monorepo with lots of individual services). Until the PR has been approved we lea…

> if the commits touch more than one of the modules (we have a monorepo with lots of individual services) That seems wrong to me - a big advantage to having a monorepo is that when you need to make a change to how two services work together, you can do so with a single atomic commit.

I may not have been clear. You can merge a single commit to master (as a fast forward rebase) regardless of how many modules it touches. But you can't do it with more than one commit. It will refuse this because we don't want non squashed stuff on master and it was an easy enough hack to prevent that. Yes it's not fool proof but works well enough so far. It discourages feature branches too which we also want. All these scripts are part of the monorepo and if I need a feature branch for something I can easily exclude that branch in these checks. I've only done that once in the last few years.

This stuff is also very fluid and changed and improved as we go. If the easy solution turns out to not work well enough for enough common cases _then_ we spend more time making it better. Otherwise 'good enough' does the trick for us. YMMV depending on your company size and developer culture. E.g. you might have too many cowboys that just add exceptions all the time or remove the checks entirely. Nobody can help you there. Fire then or flee ;)

Re: Two Years of Squash Merge (2019)

#165

Post by u801e »

Seb-C wrote: post #140
u801e wrote: post #125

Earlier quoted context omitted.

What if the change really should be split into multiple commits. Having a mega commit affecting hundreds of lines across tens of files doesn't make reviewing the change easy, and reverting it will result in conflicts.

I would ask such changes to be split into multiple independent PRs

Then you essentially double the number of commits for a particular change, since it then becomes:

1. first related change

2. first merge commit

3. second related change

4. second merge commit

5. third related change

6. ...

Instead of just having all related changes in several commits with a merge commit at the end that groups those related changes.

Re: Two Years of Squash Merge (2019)

#166

Post by tharkun__ »

da39a3ee wrote: post #131
tharkun__ wrote: post #90

Earlier quoted context omitted.

You have apparently never had the benefit of a properly run git repository. Our master has straight history. We always rebase to merge a PR and we have build scripts that check this as well. You will not be able to merge a PR that tries to get more than one commit onto master, if the commits touch more than one of the modules (we have a monorepo with lots of individual services). Until the PR has been approved we lea…

When a bug is found in production, it should usually be fixed by: 1. Make a commit supplying the test coverage that was evidently lacking. 2. Make a commit supplying the fix. Everything you say is valid. However, what I just described is the simplest and clearest example of valuable multi-commit-per-ticket git history.

I myself do and encourage my guys to do multiple commits to checkpoint their work. Also for bug fixes as you describe, first commit is the test proving its broken. This should turn the build red. Then another commit with the fix which makes the build green. All of this on the branch. PR it like that. It's valuable to see this history.

Once the PR is approved and before merging to master, rebase abd squash. The build failing intermediate commit is no longer valuable.

Re: Two Years of Squash Merge (2019)

#167

Post by u801e »

gouggoug wrote: post #137
tharkun__ wrote: post #111

Earlier quoted context omitted.

I would argue that you only loose irrelevant information and you gain the ability to rollback. Without squashing, you are actually way worse off for a "10 times a day release regimen". We release every hour and we squash and rebase with a straight master history. This enables us to almost mechanically just roll back to the previous commit that was out on Prod, should something happen and it's very easy to skip (rever…

>No guessing, no manual figuring out which 7 commits belong to the ticket in question, potentially 6 of them had the ticket number in the commit message like they should, but the 7th, [...] Lots of people it seems don't know this: _you can revert a merge commit_, which includes _all_ the commits that were part of the merge. So, if you have a feature branch with 7 commits, you merge those 7 commits _with a merge commi…

But if only 1 of the 7 commits was actually the cause of the issue, then why would you want to revert all 7 of them instead of just the one?

Re: Two Years of Squash Merge (2019)

#168

Post by tharkun__ »

u801e wrote: post #126
tharkun__ wrote: post #90

Earlier quoted context omitted.

You have apparently never had the benefit of a properly run git repository. Our master has straight history. We always rebase to merge a PR and we have build scripts that check this as well. You will not be able to merge a PR that tries to get more than one commit onto master, if the commits touch more than one of the modules (we have a monorepo with lots of individual services). Until the PR has been approved we lea…

> You will not be able to merge a PR that tries to get more than one commit onto master What value does the merge commit in the PR provide if a PR only has a single commit? Couldn't you essentially halve the number of commits in the commit history by amending the single PR commit message to contain the text that the merge commit will contain anyway?

In case it wasn't clear from my post, I am very all over the place with the word 'merge'. I usually use it to describe the concept. We don't ever have actual merge commits. We merge by rebasing and that then makes the 'merge' just a fast forward.

Re: Two Years of Squash Merge (2019)

#169

Post by tharkun__ »

u801e wrote: post #167
gouggoug wrote: post #137

Earlier quoted context omitted.

>No guessing, no manual figuring out which 7 commits belong to the ticket in question, potentially 6 of them had the ticket number in the commit message like they should, but the 7th, [...] Lots of people it seems don't know this: _you can revert a merge commit_, which includes _all_ the commits that were part of the merge. So, if you have a feature branch with 7 commits, you merge those 7 commits _with a merge commi…

But if only 1 of the 7 commits was actually the cause of the issue, then why would you want to revert all 7 of them instead of just the one?

If you have 7 commits it's very likely that 3 of those commits at least don't build because they were intermediary checkpoints. Then there's the one commit that contains the buggy code and if you revert that it breaks the whole thing and it doesn't work. Now you gotta revert the whole thing anyway, all 7 commits (or the merge).

Why bother with all this time and energy? If each single commit in your straight master history is one self contained thing it's easy to revert that whole thing and done.

We are also talking SaaS here with releases going out multiple times a day. So if something is very broken it's likely to be noticed very quickly and the breaking change is probably the last commit on master and you don't even have to revert it. You just deploy HEAD^ to Prod while you create a PR to fix the problem.

Re: Two Years of Squash Merge (2019)

#170

Post by detaro »

thom wrote: post #105
tharkun__ wrote: post #93

Earlier quoted context omitted.

I can give you an example though I have to be sort of vague for obvious reasons too. In fact, I have been bisecting to find the cause of a bug 3 times in the past 3 to 4 weeks. Customer Service reported a problem with something that my team is responsible for. I knew almost for certain that we didn't break it. I had a vague idea that another team might have broken it by a recent-ish change to a different service that…

But that just sounds like an _incredible_ amount of effort and even at the end you still don't actually know the cause of the bug. Surely you just raise it to someone who owns the code in question? It sounds like you know who they are already?

What's so incredible about spending a bit of time parallel to other things on identifying a bug?
Post reply on HN