5 ms·
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 squash
by grumple 6y ago
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.
- delecti 6y agoI 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.
- grumple 6y agoYeah, 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.
- mcv 6y agoI 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.
- devocracy 6y agoThat'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.
- grumple 6y agoAre 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.
- saagarjha 6y agoThen you split the feature up? It’s generally possible to do this.
- grumple 6y agoI 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).
- saagarjha 6y agoI actually occasionally make those kinds of pull requests and would recommend that over the alternative.