Workflow with 'never commit broken changes'

[John Arbash Meinel]

Martin Pool wrote:
> On 22 Jun 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
>> Aaron's recent branch/merge/pull question reminded me of a subtle thing
>> that the hg guys mentioned. One of the things Linus has told people is
>> "never commit broken code, because then you can't use bisect search to
>> find when a bug appears".
> 
> There are other reasons too.  If someone takes the initiative to merge
> your branch without being asked and the last commit is broken they may
> waste some time.  (Or you may forget that it's broken.)  Or someone may
> waste time trying to get it to build, thinking its their fault, when in
> fact the tree is broken.  So I make a practice of at least marking the
> commits as broken.

I agree here. I usually do try to mark specifically broken commits as such.

> 
>> It sounds like an interesting tool. I don't know if we've ever had a
>> need to find 'what revision introduced this bug'. It may just be that
>> our project is small enough that you don't have bugs cropping up in a
>> manner that the code which is causing it is unclear. (Plus we have a
>> test suite, which helps us isolate that it is very unlikely for known
>> bugs to sneak in).
> 
> I've bisected by hand along the mainline to find things, with bzr, arch
> and svn.  If you always run the test suite at checkin then you're
> typically going to be trying to find bugs that weren't known at the time
> the code was checked in.  If you discover them later and the fix isn't
> obvious then finding the change that introduced it can help you
> understand it.

One difficulty is just that if you have to run the test suite before
commit, you are adding 5-10min for the test suite to run.

> 
> I haven't looked in detail at how git bisect (which I think was the original)
> works, but what I would do is: bisect along the mainline until you find
> a change where it worked and a change where it failed.  If there are
> no merges and only text changes then one of those changes introduced the
> problem.  If there are merges coming in, then we can recurse into the
> merged revisions.
> 
>> What I was thinking about, though, is that I commit broken code fairly
>> often. I try not to, but it is fairly common that I'm working on feature
>> A, and to fix it, I need to fix feature B, which I want to commit
>> separately, but in the process I don't worry too much that the entire
>> test suite is passing.
> 
> Well, you can do it in a strictly clean way: use shelf to move away your
> changes in progress, or make a new branch, fix it there, and pull
> across.  But for many projects having every individual commit be
> strictly clean may not be worthwhile.
> 

You can do that, but I can see stacking issues. For a lot of big
refactorings, or even feature developments, you may start working on the
overall goal. And while you are going, you realize you need a helper
function. So you write the helper function along with tests, and really
that should be its own commit.
But sometimes writing one helper function actually needs another helper
function, etc.
And maybe you are just exploring something, only to expect that it isn't
going to work. So you commit, and then revert, because you may decide to
go back there, and don't really want to completely waste the last 2
hours if you change your mind again.

>> But I realized that if your bisect search was really merge aware, it
>> shouldn't actually hurt if some of the commits in a merge branch are
>> broken. Because the tip of your branch should be clean, and certainly
>> where it merges into mainline should be clean, it only matters if you
>> are the one who introduces that particular bug.
>> But it would seem the 'bisect' search should be able to localize what
>> branch brought the bug in, before it tried to find the exact revision,
>> which would give you 90% of what you want.
> 
> I suppose you could do a search which handles "good", "bad", and
> "indeterminate", and commits which are not sufficiently good to allow
> running the particular test go in the third class.
> 
>> I realize I'm trying to justify committing broken code (only in a
>> dev/personal branch). But I think there are circumstances that "NEVER
>> COMMIT BROKEN CODE", would be very restrictive. eg. I'm in the processes
>> of changing X to Y, and I've fixed 80% of the cases, but I need to go to
>> sleep. Do I have to wait until I hit 100% to add a new revision?
> 
> One option is to commit, then later uncommit and do it properly.
> 
> It also depends what you mean by "never commit broken code" - never
> commit stuff that won't compile (because then you can't test it at all),
> or never commit things that fail tests, or 
> 

I certainly commit all of the above. Again, abusing bzr as a more
intelligent rsync. I also abuse it as an Undo buffer.
Obviously I think both are a reasonable use (or I wouldn't do it). But
I'm pretty far over on the 'commit everything I do', rather than 'commit
once I get it perfect' spectrum.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060622/39413351/attachment.pgp