3 ms·
Maybe the culture of easy to understand commits exists for a reason? Imagine someone sending in a patch for the Linux kernel, or for git itself, only it isn't
by xorcist 5y ago
Maybe the culture of easy to understand commits exists for a reason?
Imagine someone sending in a patch for the Linux kernel, or for git itself, only it isn't only the patch but a bunch of temporary patches that in the end forms something usable. (But don't worry because a git feature will make the log skip over it!) Would you really want to review that? Would you really want it stored in the repository for all eternity?
Crafting commits that are easy to understand and gives enough context to understand the change is the only way a large software repository can survive without collapsing under its own historical weight.
- skinkestek 5y agoRemoving or glossing over can always be done later. Adding back the thought process that the commits can reveal can often be hard later, even for the original author.
- dolni 5y agoRemoving can't always be done later, because that requires rewriting the repository's history. Logical commits serve a real purpose: they make change review easier and can help bisect bugs later. That's orders of magnitude more useful than someone else's development thought process, which may or may not even make sense to anybody else. If thought process is important, it should be captured in a structured way like a thoughtful commit message or README addition. Not some cobbled together mess of whatever commits someone made haphazardly.
- skinkestek 5y agoFirst: are we talking past each others? You seem reasonable but some of the things doesn't make sense to me which often indicates a misunderstanding on one of the sides :-) If not, here are some clarifications. (Maybe they'll help even if we are talking past each other.) > Removing can't always be done later, because that requires rewriting the repository's history. I knew this could come up which is why I included glossed over. > Logical commits serve a real purpose: they make change review easier and can help bisect bugs later. Nothing I wrote was meant to conflict with that. Quite on the contrary I specifically want more points to bisect against (yes, I'm one of those who actually uses bisect). > If thought process is important, it should be captured in a structured way like a thoughtful commit message or README addition. Not some cobbled together mess of whatever commits someone made haphazardly. Have you tried getting anything reasonable about the thought process behind a feature into a README, and then tried to get it to stick there? On the other hand a bunchbof commits along the lines of: - rough first sketch - introduce tests. Also includes modifications to xyz to simplify testing. - performance optimizations - more tests - Introduce test for foo and fix it in bar and baz This has a negative impact on performance but is hard to avoid. etc
- shandor 5y agoCould you please elaborate on the value you find in commits from other people that are along the lines of "more tests" or "first sketch", as opposed to crafting a coherent single commit with a thoughtful commit message? I'm honestly asking in good faith, as the commits you gave as an example are literal anathema to me, and I'd like to understand what value anyone can see in them.
- skinkestek 5y agoThe one I value is the "performance optimizations" and the last one. Problem is if we let squash zealots have their way they are gone together with the rest. Storage is cheap. You'll never ever be bothered by those commits unless you are bisecting and at that point you are probably thankful of you can find a commit message along the lines of: - performance optimizations - workaround for weird problem foo problem in prod as they will tell you a lot about how to approach it. Edit: Just having tests and modification because of tests added in their own commit and with a comment ("Also includes modifications to xyz to simplify testing." above) is useful. I do look into commits on a weekly basis or more to figure out why stuff was moved / replaced. Just yesterday I stopped a PR because a constant was changed in an unrelated commit as part of the larger PR. Because commits are granular it was really easy to see that it wasn't intended.
- Hackbraten 5y ago> I specifically want more points to bisect against (yes, I'm one of those who actually uses bisect). Wouldn’t that make bisecting more difficult? If your history is full of checkpoint commits, wouldn’t bisect send you to broken commits (that you’d have to `--skip`) more often? Why would you want to deal with those extra steps?
- skinkestek 5y agoJust make sure every step make sense. One or even two or three extra bisect steps is nothing in my kind of projects as long as you get a very specific change. I guess on AAA game engines where compiles take half an hour it is more of a problem but in my case it takes 30 - 120 seconds depending on project. (I do have uncompilable WIP steps, and those I am kind of ashamed for but those are so few and so far between that it doesn't matter.)
- shandor 5y agoThanks, gonna steal using the Linux kernel development as an example regarding this matter. Probably also any huge-scale repo like Google or Windows would work, but Linux is awesome because open source. I'm honestly rather confused on the whole idea of valuing the journey instead of the end result. Aren't commit messages literally the built-in way to write about your journey to your heart's content? And failing that, you probably (should) have a ticket system or the like to preserve even more context in the form of third party comments etc. I usually compare git history to a book: it's not valuable for the end user to see all the tiny edits and errors the author made before the final version, and in the end the important thing is that it tells a good story that the end user can easily follow. Like it's said, the source code is for us humans to read. The computers would do just as fine without the story its trying to tell.