Hacker News Reader

Comment by grumple | Hacker News Reader

When you have to find a subtle one line breakage that happened years ago, you’ll be happy that you had a single commit with 30 lines of code instead of a squashed commit with 3000. It makes it much easier to isolate the case of a problem and fix it in many cases.

I’m asked “how long has this been broken and what caused it?” and answer it via git blame or bisect probably every few weeks. This is life on legacy projects.

View on HN

I think you just highlighted one source of the disagreement. Nobody on any team I work with would accept a PR with 3000 lines of change. Each team had a policy, whether formal or informal, to break large work items across PRs so they were more easily reviewable. I would say that 300 lines changed is getting towards the bigger end of what we would accept. If your choice is between 30 or 3000 lines changed per commit, then for sure I'd pick 30, but above either I think a 300 line limit is more sensible.

Yeah, that’s nice in theory, but in practice the situation is often less ideal. Some features, partially implemented, would break existing functionality if not completed, and merging those upstream prior to total completion is therefore impossible. And on the other side of things, some environments and features require large, systemic changes. This is, surely, an organizational failing, but one that we must adapt to.

Then that's the other half of my point. If you can limit PRs to 300, then merging PRs into a single commit is better. If you can't, then maybe squashing isn't ideal.

Convert all the JS files to Typescript, suddenly you get blame ed for everything.

That's not a squash or not problem, that's a your tickets are too large problem. If you normally end up with 3000 line feature commits, you're trying to do too much with individual feature changes.

Are we pretending that we, as devs, have much say in this? If our management says they want a features that’s going to take 3000 lines of code, you don’t have a choice. It’s nice if you’re somewhere where you can roll out feature mvps, but in some environments you don’t get that luxury.

And some features are just monsters, either through the nature of the feature or architectural choices that were made before it was conceived.

Then you split the feature up? It’s generally possible to do this.

I mean, I’m the one arguing for smaller commits, so yes, if you make it 10 commits via 10 prs instead, that’s fine with me. But if all of that has to go out at the same time, that’s not any different than one pr with 10 commits (and probably worse because it’s harder to see all the changes together).

I actually occasionally make those kinds of pull requests and would recommend that over the alternative.

I think based on this discussion I can identify one situation where squashing would be good: when you're doing repeated trial-and-error commits on a single file. That way, squashing won't turn it into 3000 lines of code, it just hides the unsuccessful attempts.

But other than that? Small commits please.

Reply on news.ycombinator.com