8 ms·

You misunderstand me. Almost everyone does. I do not think you should squash all of your changes into a giant hairball commit, and I don't think first parent (w

You misunderstand me. Almost everyone does. I do not think you should squash all of your changes into a giant hairball commit, and I don't think first parent (which is effectively the same thing) solves this problem either. I think each of your commits should be individually rewritten until each commit makes sense and tells a single, indivudal story that makes sense on its own, while at the same time be completely atomic and as small as possible. You created a new function? That's one commit. Take a moment to explain why this function is going to be useful in future commits. You called that new function from several new spots? That's another commit. Explain why each of these calling sites requires this function. You decided that there had to be some style and whitespace changes? That's another commit. This one can go without explanation. You found an old bug along the way that can be fixed with a one-line change? That's another commit. Hey, nice catch. Perhaps a few lines about this bug and why your small change fixes it? Together, these are all individual scenes of a larger story. But the larger story doesn't need the behind-the-scenes of how you came up with these scenes. I don't need to see all your drafts of each scene. I just want the final scenes that make up the final story.

dsego 8y ago
There is a name for what you are describing, it's called "atomic commits". Every commit can stand alone and at every commit the software should be in a working condition.
- apazzolini 8y ago
  Gary has a talk about this: https://www.destroyallsoftware.com/screencasts/catalog/source-code-history-integrity https://www.destroyallsoftware.com/screencasts/catalog/sourc...
funkaster 8y ago
that doesn't scale. I personally prefer a single squashed commit linking to a full discussion in a PR/MR: do a git blame, even if you get a giant hairball commit, you should be able to trace it to a review process (PR/MR) where it was discussed and thoroughly reviewed.
- jordigh 8y ago
  It scales quite well. Linux itself is developed in this way. Or perhaps you think Linux isn't at a large enough scale? (No sarcasm, I know that there projects out there much bigger than Linux.)
  - maksimum 8y ago
    I think the extra work for a single developer to perform atomic commits is justifiable. How does this work with multiple developers working on the same repo? I'm assuming everyone should work on their own feature branch and send PRs once their branch is done? Should the commits also be tagged by the feature branch they're on? Should the CI approval workflow be run against any combination of commits on the feature branch or against the final HEAD?
    - chousuke 8y ago
      With git, I think it helps to think of development in terms of patch series. An individual commit is a single patch, intended to do one thing (and hopefully do it well), and a feature branch is a patch series. A pull request is then a request to review the series. If you need to change things, git allows you to rewrite your commits to send in a revised set of patches. Before merging, your CI would create a temporary branch off the current master, merge your feature branch to that, and run tests against the result. I don't think testing individual commits (fully, at least) in a series makes much sense if you're going to merge all of them to master anyway.
  - markmark 8y ago
    I'm not sure about "scale" but Linux, as an open-source project where external actors want their code included, can push arbitrary amounts of work onto those actors with no additional cost to itself. They can say "to get your code upstream you have to do twice as much work" or 3x or 4x or whatever. It's not their cost to bear. An internal team pays that cost. They have to consider whether the trade off of having a pristine commit history is worth the additional overhead of doing it. I personally care more about PR size and am happy to squash all commits in a PR. If that is too big I'd rather see multiple smaller PRs.
peteforde 8y ago
While I am following this conversation closely, I wanted to politely make a suggestion about something you said: "You misunderstand me. Almost everyone does." That sounds really frustrating. It's not clear from your post whether you mean this as "everyone who reads this comment does not understand me" or "people frequently misunderstand me"... but typically, someone who feels this way experiences this in the latter, general sense. Good news: it is possible to dramatically improve the ratio of people who understand you. It will, however, require that you change how you communicate some kinds of information. What I'm saying is that if people don't understand you, the first place you need to look is your own tools for communicating with people. Of course, what you are saying makes sense to you. But there's an intellectual fallacy at work if you assume that "almost everyone" is the problem and you are the solution. :) Have you ever heard the saying, "The only thing all of your crazy ex-partners have in common is you?" There are people who are tasked with explaining far more complicated concepts than source control who people claim to love learning from. That means there is hope for us all.
- jordigh 8y ago
  Yes, I know I'm not communicating effectively. I shouldn't have implied that everyone is wrong but me. I know that there are people who understand me, but in a world of github and pull requests, it's really hard for me to explain what the world looks like otherwise.
atombender 8y ago
Ideally it should be this way, but it's impractical in reality. It requires that you either stop your development workflow to commit as you go along, or that you untangle all the pieces after they're already entangled. If you commit as you go, it's an expensive mental switch to fire up git and also run all the tests (since surely part of this workflow is to apply the principle that no commit should ever break the build). You also take an extra productivity hit every time you change your mind about something a little later (e.g. you added the function getFoo() but realize it should have been called findFoo()). If you work for a while and then try to bundle up small, atomic changes, that can also be very difficult. Tools like git group together contiguous chunks of changes when committing, and prying them apart later can be difficult. I often do this with a combination of "add -p" and then "stash save -k" to temporarily get rid of things unrelated to what I'm committing, but it's a chore. During a selective "add -p" session you have to mentally keep track of what belongs together, thus what dependencies are between every chunk you're adding. Committing as you go is easier, but it's slow, and doesn't work well when you're working across many files with a big change that introduces new semantics in a lot of places. Both techniques require that you keep track mentally of which parts are related, of course.
- jordigh 8y ago
  Untangling is what I mostly do. I consider the untangling my own internal code review. I need to read my own diff and figure out what goes where and what each part does and why it's necessary. My commit messages are then my own code review comments. I figure if I don't carefully read my own diff, why would anyone else? And once it's untangled, I am hoping others will find it easier to read too. Git doesn't provide as many tools as I would like to make this process easier. It's partly why I don't use git. Mercurial's absorb command helps a lot: it absorbs changes from your working directory into the appropriate draft commit that corresponds to the same context: https://gregoryszorc.com/blog/2018/11/05/absorbing-commit-changes-in-mercurial-4.8/ https://gregoryszorc.com/blog/2018/11/05/absorbing-commit-ch... Wait, it appears someone finally ported it to git: https://github.com/tummychow/git-absorb https://github.com/tummychow/git-absorb
  - atombender 8y ago
karmakaze 8y ago
To do this effectively, we have to change the way we edit code, so it's an editor/workflow feature rather than an scm one. Trying to solve it downstream in scm is not productive.
Ajedi32 8y ago
I mostly agree with you, but I think this might be going a little too far: > You created a new function? That's one commit. Take a moment to explain why this function is going to be useful in future commits. > You called that new function from several new spots? That's another commit. Explain why each of these calling sites requires this function. In my opinion each commit should make sense on its own. It doesn't really make sense to create an unused function, so these two changes should really be one commit.
- jordigh 8y ago
  Maybe. I think both ways can make sense, and if you insist on having the function and the calling sites in the same commit, that can make sense. I think my proposal can also make sense because (1) it splits up the commits into two units that still keeps the codebase in a stable state [this is my rough metric for what "atomic" means] and (2) defining a function requires some independent contemplation about why that function is defined that way. Inserting that function into calling sites can be a logically distinct operation if the calling sites are varied and distinct enough to each have a different reason to now require this function. Thus the two commits can be semantically distinct.
  - btschaegg 8y ago
    I see this the same way. Like the notion that purely cosmedic changes should be committed distinctly in order to not clutter up commits that change behavior (so they are simpler to read), it makes sense to me that one would commit entirely new units of code separately before re-wiring the logic of the existing code accordingly. That way the first commit basically states "I've built this new thing", and I don't have to consider 300 random existing codepoints in understanding what it does.

dsego 8y ago

There is a name for what you are describing, it's called "atomic commits". Every commit can stand alone and at every commit the software should be in a working condition.

apazzolini 8y ago
Gary has a talk about this: https://www.destroyallsoftware.com/screencasts/catalog/source-code-history-integrity https://www.destroyallsoftware.com/screencasts/catalog/sourc...

funkaster 8y ago

that doesn't scale. I personally prefer a single squashed commit linking to a full discussion in a PR/MR: do a git blame, even if you get a giant hairball commit, you should be able to trace it to a review process (PR/MR) where it was discussed and thoroughly reviewed.

jordigh 8y ago
It scales quite well. Linux itself is developed in this way. Or perhaps you think Linux isn't at a large enough scale? (No sarcasm, I know that there projects out there much bigger than Linux.)
- maksimum 8y ago
  I think the extra work for a single developer to perform atomic commits is justifiable. How does this work with multiple developers working on the same repo? I'm assuming everyone should work on their own feature branch and send PRs once their branch is done? Should the commits also be tagged by the feature branch they're on? Should the CI approval workflow be run against any combination of commits on the feature branch or against the final HEAD?
  - chousuke 8y ago
    With git, I think it helps to think of development in terms of patch series. An individual commit is a single patch, intended to do one thing (and hopefully do it well), and a feature branch is a patch series. A pull request is then a request to review the series. If you need to change things, git allows you to rewrite your commits to send in a revised set of patches. Before merging, your CI would create a temporary branch off the current master, merge your feature branch to that, and run tests against the result. I don't think testing individual commits (fully, at least) in a series makes much sense if you're going to merge all of them to master anyway.
- markmark 8y ago
  I'm not sure about "scale" but Linux, as an open-source project where external actors want their code included, can push arbitrary amounts of work onto those actors with no additional cost to itself. They can say "to get your code upstream you have to do twice as much work" or 3x or 4x or whatever. It's not their cost to bear. An internal team pays that cost. They have to consider whether the trade off of having a pristine commit history is worth the additional overhead of doing it. I personally care more about PR size and am happy to squash all commits in a PR. If that is too big I'd rather see multiple smaller PRs.

peteforde 8y ago

While I am following this conversation closely, I wanted to politely make a suggestion about something you said: "You misunderstand me. Almost everyone does." That sounds really frustrating. It's not clear from your post whether you mean this as "everyone who reads this comment does not understand me" or "people frequently misunderstand me"... but typically, someone who feels this way experiences this in the latter, general sense. Good news: it is possible to dramatically improve the ratio of people who understand you. It will, however, require that you change how you communicate some kinds of information. What I'm saying is that if people don't understand you, the first place you need to look is your own tools for communicating with people. Of course, what you are saying makes sense to you. But there's an intellectual fallacy at work if you assume that "almost everyone" is the problem and you are the solution. :) Have you ever heard the saying, "The only thing all of your crazy ex-partners have in common is you?" There are people who are tasked with explaining far more complicated concepts than source control who people claim to love learning from. That means there is hope for us all.

jordigh 8y ago
Yes, I know I'm not communicating effectively. I shouldn't have implied that everyone is wrong but me. I know that there are people who understand me, but in a world of github and pull requests, it's really hard for me to explain what the world looks like otherwise.

atombender 8y ago

Ideally it should be this way, but it's impractical in reality. It requires that you either stop your development workflow to commit as you go along, or that you untangle all the pieces after they're already entangled. If you commit as you go, it's an expensive mental switch to fire up git and also run all the tests (since surely part of this workflow is to apply the principle that no commit should ever break the build). You also take an extra productivity hit every time you change your mind about something a little later (e.g. you added the function getFoo() but realize it should have been called findFoo()). If you work for a while and then try to bundle up small, atomic changes, that can also be very difficult. Tools like git group together contiguous chunks of changes when committing, and prying them apart later can be difficult. I often do this with a combination of "add -p" and then "stash save -k" to temporarily get rid of things unrelated to what I'm committing, but it's a chore. During a selective "add -p" session you have to mentally keep track of what belongs together, thus what dependencies are between every chunk you're adding. Committing as you go is easier, but it's slow, and doesn't work well when you're working across many files with a big change that introduces new semantics in a lot of places. Both techniques require that you keep track mentally of which parts are related, of course.

jordigh 8y ago
Untangling is what I mostly do. I consider the untangling my own internal code review. I need to read my own diff and figure out what goes where and what each part does and why it's necessary. My commit messages are then my own code review comments. I figure if I don't carefully read my own diff, why would anyone else? And once it's untangled, I am hoping others will find it easier to read too. Git doesn't provide as many tools as I would like to make this process easier. It's partly why I don't use git. Mercurial's absorb command helps a lot: it absorbs changes from your working directory into the appropriate draft commit that corresponds to the same context: https://gregoryszorc.com/blog/2018/11/05/absorbing-commit-changes-in-mercurial-4.8/ https://gregoryszorc.com/blog/2018/11/05/absorbing-commit-ch... Wait, it appears someone finally ported it to git: https://github.com/tummychow/git-absorb https://github.com/tummychow/git-absorb
- atombender 8y ago

karmakaze 8y ago

To do this effectively, we have to change the way we edit code, so it's an editor/workflow feature rather than an scm one. Trying to solve it downstream in scm is not productive.

Ajedi32 8y ago

I mostly agree with you, but I think this might be going a little too far: > You created a new function? That's one commit. Take a moment to explain why this function is going to be useful in future commits. > You called that new function from several new spots? That's another commit. Explain why each of these calling sites requires this function. In my opinion each commit should make sense on its own. It doesn't really make sense to create an unused function, so these two changes should really be one commit.

jordigh 8y ago
Maybe. I think both ways can make sense, and if you insist on having the function and the calling sites in the same commit, that can make sense. I think my proposal can also make sense because (1) it splits up the commits into two units that still keeps the codebase in a stable state [this is my rough metric for what "atomic" means] and (2) defining a function requires some independent contemplation about why that function is defined that way. Inserting that function into calling sites can be a logically distinct operation if the calling sites are varied and distinct enough to each have a different reason to now require this function. Thus the two commits can be semantically distinct.
- btschaegg 8y ago
  I see this the same way. Like the notion that purely cosmedic changes should be committed distinctly in order to not clutter up commits that change behavior (so they are simpler to read), it makes sense to me that one would commit entirely new units of code separately before re-wiring the logic of the existing code accordingly. That way the first commit basically states "I've built this new thing", and I don't have to consider 300 random existing codepoints in understanding what it does.