3 ms·

What is a "semantically delineated commit"? What is a "clean history"? Why are these two things important?

by sidlls 3y ago

FeepingCreature 3y ago
Not parent: there are technical commits, such as "fix review", "fix jenkins", "fix typo" etc. Those don't delineate a particular feature but a fix for a problem that arose from the workflow. This ends up with a history of "big feature commit that is wrong in three trivial ways" + "fix 1" + "fix 2" + "fix 3". Of those, "big feature commit" is the important one, but "fix 3" is the only working one. This is clearly silly; you should pretend you were perfect from the start and squash "fix 1" through "fix 3" into "big feature commit". Your typos and brainfarts are not of historical relevance.
- sidlls 3y ago
  Perhaps I'm missing something, but I don't see how your comment answers my questions. Do you mean that a "clean history" is one without "fix 1", "fix 2" and "fix 3"? Or is that a "semantically delineated commit"?
  - ruds 3y ago
    A clean history is one where there is a single commit, "big feature commit", that produces a worktree that is the same as the one produced by "fix 3" in the "unclean" history.
    - nomel 3y ago
      How is this possible, while sharing code? Doesn't this require that pushed code is perfect? What about everyone else working on the same code? Do they wait until you've reached perfection? Or, do you squash the branch once it's complete, with the assumption that there's no other development on/from that temporary branch (I envy you if so)? (I ask these questions fully assuming I'm doing it wrong.)
      - dahart 3y ago
        > Doesn't this require that pushed code is perfect? We aren’t talking about pushed code. We are talking about cleaning up the local commit history before pushing it into a shared branch.
        nomel 3y ago
        What about the context where one works with other people, while sharing code?
        hhjinks 3y ago
        You fix your local tree before sharing it. Alternatively, you can communicate with your team and tell them they'll need to run git fetch && git rebase -i origin/main to drop your erroneously merged commits.
        dahart 3y ago
        When working with published/shared branches with other people, the advice with git has always been that history is history and not to be changed after publishing, unless there is an emergency like a security incident. Aside from that we need might need to clarify what the question is. With shared code & git, it’s nice to use a branch & merge workflow, and it’s nice to make incoming merges as clean / nice as you can do the resulting history is as smooth as it can be while capturing what happened at a reasonable granularity. These are today’s conventions though, and it’s really up to the team to decide how to balance shared work, and what people feel are the most important workflows and tools.
        sidlls 3y ago
        And that's the one--and only--reasonable use of rebasing, to squash commits from a branch before merging into main. If engineers find themselves using rebase in any other context than squashing a merge, it's time to re-evaluate the processes/culture around workflow.
- Izkata 3y ago
  They are. Or at least can be. Typos I probably agree with, but I've seen plenty of logic bugs introduced in those "fix" commits and keeping them separate from the big one is useful when figuring out what was supposed to happen.
smw 3y ago
A bunch of wip wip2 wip3 commits don't add any value, and make the log harder to read. But if you break a bigger PR down into "added feature x", "tests for feature x", "refactored y to support x" -- the commits are easier to read and provide valuable "why" history when you're trying to figure out what happened two years later.
- sidlls 3y ago
  That's more about the contents of the merged commits than anything else. Modifying the commit message(s) fixes that, as long as that's what the commits actually did. Aside from that, how are "a fix for a bug" style commits not "clean"? If merge 123 into master contains a bug that is fixed in a future merge 1234, it doesn't seem "dirty" to me; quite the opposite actually, as it tracks what actually happened. Now, "wip" style commits shouldn't be on whatever main branch everyone is working on: that's what branches are for. And if everyone is just working off the main branch and committing directly to it, that's an organizational deficiency; not one that VCS can solve.
  - dahart 3y ago
    Modifying commit messages is rewriting history, right? > “wip” style commits shouldn’t be on whatever branch everyone is working on Agreed! We aren’t talking about rewriting shared branch history, we are talking about removing the “wip” commits made hastily and locally before pushing them. Sounds like we agree!

What is a "semantically delineated commit"? What is a "clean history"? Why are these two things important?

by sidlls 3y ago

What is a "semantically delineated commit"? What is a "clean history"? Why are these two things important?

Not parent: there are technical commits, such as "fix review", "fix jenkins", "fix typo" etc. Those don't delineate a particular feature but a fix for a problem that arose from the workflow. This ends up with a history of "big feature commit that is wrong in three trivial ways" + "fix 1" + "fix 2" + "fix 3". Of those, "big feature commit" is the important one, but "fix 3" is the only working one. This is clearly silly; you should pretend you were perfect from the start and squash "fix 1" through "fix 3" into "big feature commit". Your typos and brainfarts are not of historical relevance.

sidlls 3y ago
Perhaps I'm missing something, but I don't see how your comment answers my questions. Do you mean that a "clean history" is one without "fix 1", "fix 2" and "fix 3"? Or is that a "semantically delineated commit"?
- ruds 3y ago
  A clean history is one where there is a single commit, "big feature commit", that produces a worktree that is the same as the one produced by "fix 3" in the "unclean" history.
  - nomel 3y ago
    How is this possible, while sharing code? Doesn't this require that pushed code is perfect? What about everyone else working on the same code? Do they wait until you've reached perfection? Or, do you squash the branch once it's complete, with the assumption that there's no other development on/from that temporary branch (I envy you if so)? (I ask these questions fully assuming I'm doing it wrong.)
    - dahart 3y ago
      > Doesn't this require that pushed code is perfect? We aren’t talking about pushed code. We are talking about cleaning up the local commit history before pushing it into a shared branch.
      - nomel 3y ago
        What about the context where one works with other people, while sharing code?
        hhjinks 3y ago
        You fix your local tree before sharing it. Alternatively, you can communicate with your team and tell them they'll need to run git fetch && git rebase -i origin/main to drop your erroneously merged commits.
        dahart 3y ago
        When working with published/shared branches with other people, the advice with git has always been that history is history and not to be changed after publishing, unless there is an emergency like a security incident. Aside from that we need might need to clarify what the question is. With shared code & git, it’s nice to use a branch & merge workflow, and it’s nice to make incoming merges as clean / nice as you can do the resulting history is as smooth as it can be while capturing what happened at a reasonable granularity. These are today’s conventions though, and it’s really up to the team to decide how to balance shared work, and what people feel are the most important workflows and tools.
      - sidlls 3y ago
        And that's the one--and only--reasonable use of rebasing, to squash commits from a branch before merging into main. If engineers find themselves using rebase in any other context than squashing a merge, it's time to re-evaluate the processes/culture around workflow.
Izkata 3y ago
They are. Or at least can be. Typos I probably agree with, but I've seen plenty of logic bugs introduced in those "fix" commits and keeping them separate from the big one is useful when figuring out what was supposed to happen.

smw 3y ago

A bunch of wip wip2 wip3 commits don't add any value, and make the log harder to read. But if you break a bigger PR down into "added feature x", "tests for feature x", "refactored y to support x" -- the commits are easier to read and provide valuable "why" history when you're trying to figure out what happened two years later.

sidlls 3y ago
That's more about the contents of the merged commits than anything else. Modifying the commit message(s) fixes that, as long as that's what the commits actually did. Aside from that, how are "a fix for a bug" style commits not "clean"? If merge 123 into master contains a bug that is fixed in a future merge 1234, it doesn't seem "dirty" to me; quite the opposite actually, as it tracks what actually happened. Now, "wip" style commits shouldn't be on whatever main branch everyone is working on: that's what branches are for. And if everyone is just working off the main branch and committing directly to it, that's an organizational deficiency; not one that VCS can solve.
- dahart 3y ago
  Modifying commit messages is rewriting history, right? > “wip” style commits shouldn’t be on whatever branch everyone is working on Agreed! We aren’t talking about rewriting shared branch history, we are talking about removing the “wip” commits made hastily and locally before pushing them. Sounds like we agree!