Workflow with 'never commit broken changes'

[Michael Ellerman]

On 6/23/06, 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".

He probably said that WRT bisect, but I think it's basically been
policy from just about day one. Because the kernel started out as, and
still is to a large extent, a patch swapping community, it just
doesn't really make sense to send someone a patch which doesn't work.

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

It isn't interesting, it's crack. In the sense that once you've used
bisect to find a bug you will never go back to a VCS that doesn't
support it. You'll be hooked.

I think you're right about bzr, it's just not big/complicated enough
yet for bisect to be that important, and the test suite helps a lot.
But for something like the kernel, where a change in generic code
might break an obscure arch configuration that someone tries to boot 3
months later, it's invaluable.

Even then, I think it would be good to use on bzr. If you can automate
the bisection, it should be quite fast, and it allows you to find the
exact commit that broke something. So rather than saying "something in
joe blogs' 400 commit foo branch broke x", you can say "commit
joe at bar.org-8383-3838 broke x". Which I think is valuable information.

> 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.
> Or I've been working on my laptop, an I want to move over to my desktop.
> Robert mentioned this is abusing bzr as a substitute for rsync/unison.
> He has a point, but I also know that rsync may through away uncommitted
> changes on my desktop (or even committed changes that I don't have).

For that case I'd commit, push, then uncommit on both sides. It's a
bit of a pain, but better IMHO. I notice there's a new bzrtools
command called "shove" which caters to this use case even better.

> We never commit things to the mainline which is broken (which I do
> support). But I have to wonder what broken commits on side branches
> might do for something like 'bisect'.

But why the distinction? If things only work on the mainline, that
might leave you trawling for a bug introduced in a 10,000 line-diff
branch merge.

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

Never say never :) But in general I think it's a very good rule to try
and follow. At the same time there are definitly advantages to
committing early and often, so it's a trade off.

What I think would be good would be a way to tag a revision as
broken/working, then you could have the bisect tool be aware of that
and skip them. That'd free you up from committing broken code, with
the knowledge that you won't pay for it later.

> I'm not a kernel hacker, so maybe I'm missing some true enlightenment.
> Creating a dev process around your tools is required to some extent
> (human beings are generally more flexible than any tool). But
> ultimately, you should try to build your tools around the process you
> want to use.

I think it's mainly a matter of scale, it's just nasty to have the
kernel deliberately broken at points on its history. There's enough
bugs as it is without developers knowlingly committing code which
doesn't work.

cheers