Workflow with 'never commit broken changes'

[Martin Pool]

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.

> 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.

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.

> 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 

-- 
Martin