4 ms·

Split up your changes. Seriously: in most cases it's possible to split up a 30-line patch into 5 separate patches that, applied in order, monotonically improve

Split up your changes. Seriously: in most cases it's possible to split up a 30-line patch into 5 separate patches that, applied in order, monotonically improve your code base and are easier to review, in total, than one larger patch. Changesets are cheap, and we should be optimizing them for easy review, so any eyeball we get can see what's going on.

morsch 15y ago
When I'm ready to commit, I usually have solved the problem in code. In order to create five separate patches that illustrate the line of thinking, wouldn't I have to go back in time and re-create the intermediary steps? I suppose I could create patches as I am actively solving the problem, but at that point my code may very well be a mess that I'd have to clean up each time. Of course all of that is moot if your problem naturally segments into several patches, if nothing else than simply by the virtue of being larger than a 30-line patch or involving several mostly independent components.
- cpeterso 15y ago
  If you've completed a big change and can't stage it in separate commits that build upon each other ("telling a story" of the feature development), I recommend at least splitting non-overlapping chunks that can stand (compile/test) independently into their own commits. git-cola is a good GUI to visually stage chunks into separate commits. I haven't really used git-cola's other functionality, but I really like its visual staging features. http://git-cola.github.com/screenshots.html http://git-cola.github.com/screenshots.html
  - drothlis 15y ago
    git-gui (part of the core git distribution) provides similar mousey-clicky staging -- you can stage hunks or individual lines at a time.
- drothlis 15y ago
  Basically, yes. It does mean more work for you, for the benefit of the reviewers of the code. Arguably, it is more important to optimise for the readers of the change (many people, spread over time) than for the writer (one person, once). If you use, say, git, you might commit every single, tiny change separately, on a private (local) branch; then use `git rebase --interactive` to reorder/merge the commits as necessary. This is the easiest way I have found, but it still involves more work. If you wanted to take it further, you could have your editor automatically commit on every save, with a post-commit hook that takes your changes into a staging area, compiles/tests, and provides a report for each commit.
InclinedPlane 15y ago
I wonder if there's a sort of perverse incentive happening where devs try to avoid checkins that are "too simple" or "not meaty enough". It seems like a similar phenomenon to avoidance of using the extract method refactoring even down to the limit of one line methods.
gruseom 15y ago
It isn't necessarily easier to understand five simpler patches than one more complex patch. That is the same argument that makes people believe that short functions are always better. It's a bookkeeping trick: allocate part of the cost to something you consider irrelevant (e.g. the incremental cost of a commit or of defining and calling a function) and it seems like total complexity has gone down when in reality it has often gone up somewhat.
- akkartik 15y ago
  It's easier to understand five simpler patches if 'diff' is too stupid to render the more complex patch. "Here I renamed this variable, then I moved this hunk over there, then I made the change." Even aside from the artifact of diff, I don't think this situation is analogous to short functions at all. The big difference is that commit logs are much more serial in nature. If I break one commit down to a sequence of 5 simpler commits they will almost always be read in that order, with context preserved. I'm ambivalent about long functions, but am much more draconian about complex commits: http://github.com/akkartik/wart http://github.com/akkartik/wart
  - gruseom 15y ago
    Largely agree, but it's still a tradeoff. Having many fine-grained commits makes individual steps more intelligible but the overall path harder to make out.
    - akkartik 15y ago
      Does anybody actually use commit histories to eyeball the evolution of a project? I have some experience browsing them to find simpler-to-understand snapshots, but that's a much more incremental idea, and even that is usually met with blank looks. If there were people doing this, I'd see the odd little tool to make it easier, to annotate logs and so on. But there are no signs of this. If nobody is looking to commit histories for narrative, perhaps it's because the commit history is the wrong place for it. Since history is immutable it's really hard to have a coherent narrative of a project. Trying to do that ends up with commit messages like "final version", "really final version", "final version this time for sure", etc. My attitude has been to leave the narrative up to the reader to reconstruct. All I can do is talk about this particular point in time. Hmm, even if the globally coherent narrative is impossible, perhaps it's worth trying to keep a piecewise-coherent history. I tend to have 'section boundaries' where I start a new feature/subsystem/narrative[1]. Perhaps I should demarcate them with "=== " or something so they're easier to see in the log. [1] Again, I never attempt to demarcate where a feature or narrative ends, because that's impossible to judge without hindsight.
__david__ 15y ago
I tend to have different techniques for different situations. For instance, when wrapping an existing chunk of code in an "if" statement or "while" loop, I'll check in the initial change without re-indenting the existing code. Maybe I'll half-indent the new parts or maybe not indent them at all. Either way, that makes the diff readable. Then I'll do a re-indent and check that in, but to me that's ok since it's just a "whitespace" checkin.
- drothlis 15y ago
  We have `diff -w` for this type of thing (and `git diff -w` etc). Sadly, we're still missing `git annotate -w`.

morsch 15y ago

When I'm ready to commit, I usually have solved the problem in code. In order to create five separate patches that illustrate the line of thinking, wouldn't I have to go back in time and re-create the intermediary steps? I suppose I could create patches as I am actively solving the problem, but at that point my code may very well be a mess that I'd have to clean up each time. Of course all of that is moot if your problem naturally segments into several patches, if nothing else than simply by the virtue of being larger than a 30-line patch or involving several mostly independent components.

cpeterso 15y ago
If you've completed a big change and can't stage it in separate commits that build upon each other ("telling a story" of the feature development), I recommend at least splitting non-overlapping chunks that can stand (compile/test) independently into their own commits. git-cola is a good GUI to visually stage chunks into separate commits. I haven't really used git-cola's other functionality, but I really like its visual staging features. http://git-cola.github.com/screenshots.html http://git-cola.github.com/screenshots.html
- drothlis 15y ago
  git-gui (part of the core git distribution) provides similar mousey-clicky staging -- you can stage hunks or individual lines at a time.
drothlis 15y ago
Basically, yes. It does mean more work for you, for the benefit of the reviewers of the code. Arguably, it is more important to optimise for the readers of the change (many people, spread over time) than for the writer (one person, once). If you use, say, git, you might commit every single, tiny change separately, on a private (local) branch; then use `git rebase --interactive` to reorder/merge the commits as necessary. This is the easiest way I have found, but it still involves more work. If you wanted to take it further, you could have your editor automatically commit on every save, with a post-commit hook that takes your changes into a staging area, compiles/tests, and provides a report for each commit.

InclinedPlane 15y ago

I wonder if there's a sort of perverse incentive happening where devs try to avoid checkins that are "too simple" or "not meaty enough". It seems like a similar phenomenon to avoidance of using the extract method refactoring even down to the limit of one line methods.

gruseom 15y ago

It isn't necessarily easier to understand five simpler patches than one more complex patch. That is the same argument that makes people believe that short functions are always better. It's a bookkeeping trick: allocate part of the cost to something you consider irrelevant (e.g. the incremental cost of a commit or of defining and calling a function) and it seems like total complexity has gone down when in reality it has often gone up somewhat.

akkartik 15y ago
It's easier to understand five simpler patches if 'diff' is too stupid to render the more complex patch. "Here I renamed this variable, then I moved this hunk over there, then I made the change." Even aside from the artifact of diff, I don't think this situation is analogous to short functions at all. The big difference is that commit logs are much more serial in nature. If I break one commit down to a sequence of 5 simpler commits they will almost always be read in that order, with context preserved. I'm ambivalent about long functions, but am much more draconian about complex commits: http://github.com/akkartik/wart http://github.com/akkartik/wart
- gruseom 15y ago
  Largely agree, but it's still a tradeoff. Having many fine-grained commits makes individual steps more intelligible but the overall path harder to make out.
  - akkartik 15y ago
    Does anybody actually use commit histories to eyeball the evolution of a project? I have some experience browsing them to find simpler-to-understand snapshots, but that's a much more incremental idea, and even that is usually met with blank looks. If there were people doing this, I'd see the odd little tool to make it easier, to annotate logs and so on. But there are no signs of this. If nobody is looking to commit histories for narrative, perhaps it's because the commit history is the wrong place for it. Since history is immutable it's really hard to have a coherent narrative of a project. Trying to do that ends up with commit messages like "final version", "really final version", "final version this time for sure", etc. My attitude has been to leave the narrative up to the reader to reconstruct. All I can do is talk about this particular point in time. Hmm, even if the globally coherent narrative is impossible, perhaps it's worth trying to keep a piecewise-coherent history. I tend to have 'section boundaries' where I start a new feature/subsystem/narrative[1]. Perhaps I should demarcate them with "=== " or something so they're easier to see in the log. [1] Again, I never attempt to demarcate where a feature or narrative ends, because that's impossible to judge without hindsight.

__david__ 15y ago

I tend to have different techniques for different situations. For instance, when wrapping an existing chunk of code in an "if" statement or "while" loop, I'll check in the initial change without re-indenting the existing code. Maybe I'll half-indent the new parts or maybe not indent them at all. Either way, that makes the diff readable. Then I'll do a re-indent and check that in, but to me that's ok since it's just a "whitespace" checkin.

drothlis 15y ago
We have `diff -w` for this type of thing (and `git diff -w` etc). Sadly, we're still missing `git annotate -w`.