4 ms·
> A commit or a PR can be large. I don't see the difference. They made it pretty clear they're talking about not-large commits. And they're contrasting that w
by Dylan16807 2y ago
> A commit or a PR can be large. I don't see the difference.
They made it pretty clear they're talking about not-large commits. And they're contrasting that with any-size PRs.
- EnPissant 2y agoThat's a false dichotomy, lurker. A PR or a commit can be large or small.
- Dylan16807 2y agoIt's not a false dichotomy. They're just using different terms than you would, based on their experience with how the people around them use those systems.
- EnPissant 2y agoIt's comparing carefully manicured git history hacking to unregulated PRs. It's pure dogma.
- Dylan16807 2y agoYou agree that they are making a real comparison between two different things. But you also say it's pure dogma? I'm confused.
- EnPissant 2y ago>I loathe GitHub PRs because of this. Working at $dayjob the unit of change is the commit, and every commit is reviewed and signed off by at least 1 peer. Because this is exactly what a squash merged PR is. There is no meaningful difference unless you say "but commits are done by good people and PRs are done by bad people".
- Dylan16807 2y agoThey make it very clear that they are praising small commits, and squash merge commits are usually not small. Squashing is the opposite of what they want. The preference they have is not exactly a problem with github PRs, but github PRs are much more likely to review a big pile of code at once. The amount of code being reviewed at once is a meaningful and extremely objective measure, and that's the thing they're concerned with. Not who made it.
- EnPissant 2y agoThere is zero chance that any shop that reviews individual commits is not squashing them. I probably wont be able to respond to any more comments. Dang put a slowban on my account because he interpreted one of my comments as right wing.
- Dylan16807 2y agoWell then you're calling them a liar, I guess. But even if they're lying about achieving it, the preference they have for reviewing small commits is a preference that makes sense. It's not some nonsense "us versus them" thing.
- sfink 2y agoAt Mozilla, we review individual commits and do not squash them. This is probably true of anyone using one of the forges capable of handling patch stacks. ("Probably" because the shop may or may not review everything!) Once in a great while, I will keep the commits separate for review and squash for landing, but that's because I intentionally left things in a half-complete state for ease of review. When you're looking back at why something was done in a certain way, the review view is more useful than either the squashed view or the stream-of-work view. A human put effort into making it understandable, so it's no surprise that it's more understandable.
- EnPissant 2y agoSo if someone reviews your changes and you both agree to rename a variable, is that its own commit or do you squash it?