Live data from Hacker News

Two Years of Squash Merge (2019)

blog.dnsimple.com

181–190 of 194 posts

Re: Two Years of Squash Merge (2019)

#181

Earlier quoted context omitted.

hah, a big merge. in the next month or so, i will need to merge a branch with nearly 500 commits that change more that 15k lines of code, under development for more than a year. but yeah, like your "one big merge", i do not intend to squash it (though there are actually some arguments in favor that only kick on with a merge of this size).

I would argue that this feature branch of yours is way too long lived :) But I get it, sometimes you can't change some things in a corporate environment. I would like to know though, if those 500 commits are all individual PRs/tickets that originally might have been 3798 commits but each PR that got merged into the feature branch was squashed? Or are these 500 individual commits that are potentially interleaved, mean…

I'm not in a corporate environment. ardour.org is it.

The 500 commits cover a development process that radically alters a fundamental data representation within the software. It was difficult to design, difficult to implement, difficult to test. The whole process has simply taken a long time, and we've had other orthogonal development taking place in master.

Re: Two Years of Squash Merge (2019)

#182

Earlier quoted context omitted.

I would argue that this feature branch of yours is way too long lived :) But I get it, sometimes you can't change some things in a corporate environment. I would like to know though, if those 500 commits are all individual PRs/tickets that originally might have been 3798 commits but each PR that got merged into the feature branch was squashed? Or are these 500 individual commits that are potentially interleaved, mean…

I'm not in a corporate environment. ardour.org is it. The 500 commits cover a development process that radically alters a fundamental data representation within the software. It was difficult to design, difficult to implement, difficult to test. The whole process has simply taken a long time, and we've had other orthogonal development taking place in master.

You are definitely in a different world. As a dev I really like SaaS as incremental changes are much easier to do. I bet you've weighed the pros and cons of making those changes incrementally vs. as a big bang change.

Re: Two Years of Squash Merge (2019)

#183
post #94

Earlier quoted context omitted.

Can't you achieve something similar with `--no-ff` and tags?

A tag defines an endpoint, not a group of related commits. Yes, I could roll my own adhoc layer on top of git but it wouldn't have any tooling support. "Well, you see you can infer the start of the rangE of commits for this feature by going back to the previoNOOOOOPE

Leaving branches around would also work depending on the adhoc implimentation. I thought tags would be nicer as they'd be less than leaving thousands of branches.

> Yes, I could roll my own adhoc layer on top of git but it wouldn't have any tooling support.

Most git commands are ad hoc shell scripts built on git's internal data types. I can't see why adding your own would be that bad, a lot of external git tooling does this.

Re: Two Years of Squash Merge (2019)

#184
post #165
post #140

Earlier quoted context omitted.

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.

The number of commits does not matter. What matters is the quality of it. My opinion is that each PR should be an independent and working unit.

If you have a too-huge PR that can be subdivided, it almost always means you have multiple units of work (feature/fixes) into one branch/PR.

The reason I agree with the article and use squash is that it allows us to have a different history between master and the local branches. Developers can freely subdivide each feature into how many commits are useful to their mindset and workflow, while the master branch will remain clean.

In my experience, the individual scope of each commit on the master branch is almost always correlated to the scope of one code review, and thus one PR.

Re: Two Years of Squash Merge (2019)

#185
post #175

Earlier quoted context omitted.

One can ensure that every commit passes the tests suite and linting by running something like: git rebase --exec "test_cmd" --exec "lint_cmd" base_branch and any commit that doesn't pass can be fixed until you get to the point where all commits in the branch pass. Then you can find the commit that caused the issue, revert it and then make another commit that fixes the issue in the next PR (and ensure that the test su…

I personally think that individual commits are mine and nobody should care. They help me. They're not meant to live forever and if you make me make all of them even compile let alone build green then I will squash before ever pushing. A red build should result from the first commit on a bug fix if you ask me. I think we just fundamentally think differently. Your branches seem much longer lived. And each of your commi…

> I personally think that individual commits are mine and nobody should care. They help me. They're not meant to live forever

But they don't really help when reviewing because they're hard to make sense of. But if you want to make a large change in a single commit, then why not just stage the entire thing as a single commit in the first place. You can address review comments by amending the commit and force pushing it.

By breaking it down into sensible commits, it makes it easier for the reviewer to review your change by filtering it by commit, so that they can see a subset of it.

> You also deliver smaller chunks of changes so customers can give you more fine grained feedback

Some of those changes involve some degree of refactoring. Typically, I'll separate the refactor part from the implementation part as separate commits. If I were to just make a PR that just did refactoring, then how would I justify merging it and deploying it to production?

Also, making a bunch of small changes and merging them separately takes away the association between them, unlike what you get by keeping multiple related commits in a branch.

Re: Two Years of Squash Merge (2019)

#186
post #184
post #165

Earlier quoted context omitted.

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.

The number of commits does not matter. What matters is the quality of it. My opinion is that each PR should be an independent and working unit. If you have a too-huge PR that can be subdivided, it almost always means you have multiple units of work (feature/fixes) into one branch/PR. The reason I agree with the article and use squash is that it allows us to have a different history between master and the local branch…

Then there really is no need to even have merge commits. But, as far as I know, there no way to disable the creation of a merge commit when using the merge button on the github PR page.

The one thing you lose with just making one PR per commit is the relationship between a set of commits used to implement a feature. One commit involves some retractoring to make it easier to cleanly implement the feature. The other commit would be the feature and tests, and the last commit would be to add calls/references to that feature.

If these commits were kept in the same branch and merged in a single PR, then it would be easy to see the relation between them. If they were merged as 3 separate PRs, then it would be more difficult to see why the refactoring was done, especially if PRs for unrelated changes were merged in the interim.

Re: Two Years of Squash Merge (2019)

#187

Earlier quoted context omitted.

> - 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, bisectin…

Can't you just bisect twice. Once to find the PR and a second time within the PR's branch?

AFAIK, if you squash merge, the SCM (intended as the repository, both local and remote) will keep only a single commit; the commits breakdown will be in GitHub's (intended as a service) history.

Re: Two Years of Squash Merge (2019)

#188
post #185

Earlier quoted context omitted.

I personally think that individual commits are mine and nobody should care. They help me. They're not meant to live forever and if you make me make all of them even compile let alone build green then I will squash before ever pushing. A red build should result from the first commit on a bug fix if you ask me. I think we just fundamentally think differently. Your branches seem much longer lived. And each of your commi…

> I personally think that individual commits are mine and nobody should care. They help me. They're not meant to live forever But they don't really help when reviewing because they're hard to make sense of. But if you want to make a large change in a single commit, then why not just stage the entire thing as a single commit in the first place. You can address review comments by amending the commit and force pushing i…

I make individual commits for addressing review comments. It's sometimes helpful to see this during further review both for existing or new reviewers. They get squashed like any others afterwards. I don't see how it would be valuable after something is merged to master that I renamed a bunch of variables, extracted a method somewhere and added one test after a reviewer commented that I missed a case.

Refactorings are sometimes done with a separate PR and sometimes not. Depends on the size. From our discussion we might have different philosophies on when to do that. And yes at our company merging and deploying a refactoring to master is totally acceptable and done frequently. Even if the ticket takes longer in the end, gets scrapped etc. the refactoring can probably stand on its own and be valuable if we extracted it to its own PR.

Association should be there through tickets. We may have a different idea of what small is and what will be in such a commit. Let's have a made up example:

You are building something that has a table view for some data. That feature is out there. But the table doesn't have sorting capabilities and no filtering and no paging. It just displays everything.

There are individual tickets for adding each of these. Sorting is a small PR because the table component you use actually has that capability. You add the flag to enable sorting and the definition for the default sort and release it.

Filtering is another ticket. Filtering is harder. The API this is based on doesn't support filtering and isn't owned by your team. If this had been part of the same feature branch and released together customers would have nothing yet. Instead they have sorting already. You decide to do filtering via API and not in the UI after fetching everything. So off to create a ticket for the other team and talking to them. Maybe you can help out and do it for them but in any case this will take a while.

While waiting for their answer you move on to Paging! Yet another ticket. You notice that your table component doesn't support infinite scrolling. You want to have that though instead of individual paging. You create two PRs. One to the table component which adds infinite scrolling. This is an individual PR that is regression tested against all the other users of it in your code base. Once that's done you merge it. Why wait? Then you PR the actual change to use infinite scrolling with your specific table. This is at this point still based on that API that doesn't do filtering so you still always retrieve all data. But with lots of data it still helps rendering speed and the API is reasonably fast even with thousands of rows.

You create a new ticket to clean this up and make it work by retrieving paged data once the API also does filtering. Or maybe you never will.

Re: Two Years of Squash Merge (2019)

#189
post #185

Earlier quoted context omitted.

> I personally think that individual commits are mine and nobody should care. They help me. They're not meant to live forever But they don't really help when reviewing because they're hard to make sense of. But if you want to make a large change in a single commit, then why not just stage the entire thing as a single commit in the first place. You can address review comments by amending the commit and force pushing i…

I make individual commits for addressing review comments. It's sometimes helpful to see this during further review both for existing or new reviewers. They get squashed like any others afterwards. I don't see how it would be valuable after something is merged to master that I renamed a bunch of variables, extracted a method somewhere and added one test after a reviewer commented that I missed a case. Refactorings are…

> I make individual commits for addressing review comments.

Those changes are visible when you make a force push in github since it generates a link that shows those changes.

> Association should be there through tickets.

I've seen companies change ticketing systems several times in career. Once it changes, all the old links and associations are as good as gone. But if that association is maintained via git, then that's not an issue.

Re: Two Years of Squash Merge (2019)

#190
post #189

Earlier quoted context omitted.

I make individual commits for addressing review comments. It's sometimes helpful to see this during further review both for existing or new reviewers. They get squashed like any others afterwards. I don't see how it would be valuable after something is merged to master that I renamed a bunch of variables, extracted a method somewhere and added one test after a reviewer commented that I missed a case. Refactorings are…

> I make individual commits for addressing review comments. Those changes are visible when you make a force push in github since it generates a link that shows those changes. > Association should be there through tickets. I've seen companies change ticketing systems several times in career. Once it changes, all the old links and associations are as good as gone. But if that association is maintained via git, then tha…

I like how you first say that doing something in git itself is not needed because a random tool you use but I don't does things so that you can still see it easily and then you turn around and tell me that something is better to be visible in git history itself only because companies change tools.

Weird.

Tell me, in your career, how many times have you seen companies switch source control tools?

I have seen it many many times. In fact I've done some of these migrations for my companies in the past.

I've seen the same with ticketing systems as well. Yes you can loose history in both these transitions. I've worked with systems where either ticket or version history were not available past a certain point. Usually viewing many many years into the past was possible because people realized that historic information can be valuable. But there was a cutoff point that balances out ROI.

Unless required for regulatory purposes maybe, who really needs commit history from 15 years ago? It's cool don't get me wrong. I loved digging through commit history on code that originally was tracked via RCS on a (at the time) ~15 year old code base. And that was about 15 years ago. I'm getting old lol!

Post reply on HN