3 ms·

Small commits where each commit represents nothing of value and doesn't compile are terrible for git bisect. And for code review. A code reviewer doesn't care

by mort96 1y ago

Small commits where each commit represents nothing of value and doesn't compile are terrible for git bisect. And for code review. A code reviewer doesn't care that you spent 10 days across 100 commits tweaking some small piece of code, the code reviewer cares about what you ended up with. The bisecter probably wants to compile the code at every commit to try and see if they can reproduce a bug. Commits which don't compile force the bisecter to step through commits one by one until they find one which compiles, linearizing a process which should be logarithmic.

deleted 1y ago
[deleted]
lmm 1y ago
> Commits which don't compile Those are bad, don't do that. (At least don't do long chains of them. The occasional isolated one is ok) I still find writing a "good" commit message to be a much heavier process than making sure that it compiles.
- mort96 1y ago
  But you disagree with the other part of my comment, or..?
  - lmm 1y ago
    I agree that: - Human readers don't care about the details of everything you tried - Commits that both don't compile and have no useful message are of very little value But I don't think that's an argument against making small commits with uninformative messages, because making those commits without breaking compilation - or even explicitly checking that it compiles before committing - is much easier than coming up with a full commit message. And small commits that do compile - or even small commits of which a significant proportion compile - are very useful for bisect.
    - mort96 1y ago
      But you don't agree that it's better if those 100 tiny commits with messages like "x" and which all just tweak the same piece of code are cleaned up with some squashing and rewording before making a PR?
      - lmm 1y ago
        > But you don't agree that it's better if those 100 tiny commits with messages like "x" and which all just tweak the same piece of code are cleaned up with some squashing and rewording before making a PR? Not unless they're literally just e.g. making a change and undoing it. In general 100 tiny commits with messages like "x" don't make human PR review any harder and they make (automated) bisect to find a bug better.

by mort96 1y ago

deleted 1y ago

[deleted]

lmm 1y ago

> Commits which don't compile Those are bad, don't do that. (At least don't do long chains of them. The occasional isolated one is ok) I still find writing a "good" commit message to be a much heavier process than making sure that it compiles.

mort96 1y ago
But you disagree with the other part of my comment, or..?
- lmm 1y ago
  I agree that: - Human readers don't care about the details of everything you tried - Commits that both don't compile and have no useful message are of very little value But I don't think that's an argument against making small commits with uninformative messages, because making those commits without breaking compilation - or even explicitly checking that it compiles before committing - is much easier than coming up with a full commit message. And small commits that do compile - or even small commits of which a significant proportion compile - are very useful for bisect.
  - mort96 1y ago
    But you don't agree that it's better if those 100 tiny commits with messages like "x" and which all just tweak the same piece of code are cleaned up with some squashing and rewording before making a PR?
    - lmm 1y ago
      > But you don't agree that it's better if those 100 tiny commits with messages like "x" and which all just tweak the same piece of code are cleaned up with some squashing and rewording before making a PR? Not unless they're literally just e.g. making a change and undoing it. In general 100 tiny commits with messages like "x" don't make human PR review any harder and they make (automated) bisect to find a bug better.