Submitting code (was Re: [PATCH] auto rename...)

[Matt Mackall]

On Fri, 2008-10-03 at 08:21 +0200, Herbert Griebel wrote:
> Matt Mackall wrote:
> > On Thu, 2008-10-02 at 17:18 +0200, Herbert Griebel wrote:
> >> Matt Mackall wrote:
> >>> On Wed, 2008-10-01 at 17:57 +0200, Benoit Boissinot wrote:
> >>>>> -static char mdiff_doc[] = "Efficient binary diff.";
> >>>>> +
> >>>>> +typedef struct {
> >>>>> +	unsigned int hist[256];
> >>>>> +	unsigned int filelen;
> >>>>> +} STATDATA;
> >>> Uh oh. Why did we grow more C code?
> >> If you mean the typedef, this has been fixed already,
> >> please see the latest patch (1 hour ago).
> > 
> > No, I mean: there appears to be a lot of new C code to support a
> > relatively obscure feature. This is maybe not a good thing as C code is
> > much more verbose, fragile, and hard to maintain.
> > 
> > There is lots of Mercurial code that perhaps should be in C for
> > performance reasons. But I don't think this is in the top 10.
> > 
> > So, please: start with everything in Python. When we've got that all
> > merged and happy, we can talk about further performance enhancements.
> 
> Patch is here:
> http://www.soundboot.com/hg/autorename_bestmatches_and_speed.patch

I think I need to take a moment and talk about "best ways to submit
code". This code is interesting, but it's just not going to make it in
like this.

The first thing everyone needs to realize is that we're all -very- busy.
We're all working on this in our spare time and just handling bug
reports can easily consume all of that time. That means we simply cannot
sit down for an hour to figure out what your code is doing. In fact, I
do not even have time to actually open all of the mail I get, never mind
read and respond to it.

A successful patch is:

a) in the BODY of the message

If I can't hit reply to your patch, I'm not going to go to the trouble
of cutting and pasting your code from a URL to give you feedback. I have
too much to do.

Likewise, if I can't drag and drop your message into my patch queue,
I'll probably postpone dealing with it effectively forever.

Patches sitting in the BTS or elsewhere are dead on arrival. Our backlog
is too long for us to deal with the stuff that's in our inboxes, let
alone elsewhere.

b) broken into digestible chunks

We don't want patches that radically change things. They're more likely
to break things and they can't be bisected. Also, they're too big to
review. And we obviously can't take patches we can't review: if the
quality of the code base drops, we'll have even less time to deal with
contributions.

Ideally we want a series of patches that each make one small step
towards the goal. Each patch should leave the code in a working state
and take less than five minutes to look over. Again, if it takes me more
than a few minutes to deal with, it'll go into the backlog.

If you can, put the least controversial bits first. If you're lucky,
someone will apply those right away and you'll be that much closer to
being done.

c) include all the relevant context

If you're resending a patch that I commented on last week, it's safe to
assume I've forgotten everything about it. In the intervening times I've
looked at dozens of other patches and bugs, spent lots of time working
on unrelated projects, and maybe gone to a conference. 

So if your patch says "here is a resend of the patch from last week with
some changes", I will have absolutely no idea what that means. Rather
than grovelling through thousands of messages in my received mail folder
to figure out what your patch is about, I'll probably just put your
patch on the "to look at later" pile.

So tell me everything you want me to know about your patch. Every time.
Preferably in the patch's changelog. I want to know how it works, what
the new output looks like, why you did it the way you did and not some
other way, *what feedback you've incorporated*, etc. If your patch is a
reasonable size, this should still be a short message and I should be
able to read the whole thing and reply to it or apply it in five
minutes.

d) try not to overwhelm us

If you're not being successful with getting your entire series of
patches in all at once, you may be more successful sending just the
first small pieces. You're more likely to make headway, and we'll have a
lot less mail.

You'll also find it's very helpful to talk to us on IRC about getting
your patch in. We can often give you realtime feedback on any blocking
issues.

-- 
Mathematics is the supreme nostalgia of our time.