[Libvirt-cim] Best practices for patchsets

[Jay Gagnon]

Lately we've been having more groups of patches than single patches,
which I believe to be a good thing because it also means we've been
getting less 500 line patches entitled "Fix everything." :)  However,
it's also increasing our opportunity for confusion, as we have more
opportunity for patchset fragmentation.  I have a couple of suggestions
for how we should handle patchsets, along with a few things that have
been floating around my head for a while about patches in general, so
I'm laying them out here.  I'd like to know what everybody thinks, and
eventually get the final version somewhere on the website as well.

Patches in general:

1. When you add a patch to the queue you have an idea of where you're
going with it, and the commit message should reflect that.  Be
specific.  We have a tendency to say something like "Various fixes to
AllocationCapabilities" (I'm a huge offender here), when we really
should be saying what was actually fixed, like "add EnumInstances
support" or "eliminate duplicate instances."

2. Stay on task with a patch.  If you notice other problems while you
are working on patch, and they are not most definitely specific to your
patch, they should be another patch.

3. This will sound a lot like "Stay on task," but resist the temptation
to nitpick with a patch.  I have a peculiar desire to have all declares
at the beginning of the function, sorted according to length in
ascending order.  We all have our own idea on how best to name a
variable or format a function call.  Often, the change is in fact
worthwhile, and is welcome, but lots of little changes that don't
directly relate to a fix can really make that patch hard to read.  They
should be another patch.

4. Before you type, "hg email," you should always type, "hg export,"
first.  Review your patch.  Does it have any typos in the comments?  Did
you accidentally include an irrelevant change?  Is your commit message
still accurate and useful?


Patchsets:

1. When you send a group of patches, Mercurial's email extension will
create a "header" email.  Make the subject and body of that email
meaningful, so we know how the patches relate.  It's easy to say, "Each
patch has a commit message, it's obvious how they work together," but
the rest of the list usually won't agree with that.  If the commit
messages for each patch are good, you shouldn't need more than a
sentence or two to tie them all together, but you do need it.

2. If any of your patches are rejected and you rework them, resend the
entire set.  This prevents things like, "So use patch 1 of 4 from the
set I sent yesterday, 2 and 3 of 4 from the patch I sent an hour later,
and patch 4 of 4 from today."

3. If you resend a patchset, and the "Patch [0 of 3]" subject line was,
"AllocationCapabilities fix," then the new subject line should be,
"AllocationCapabilities fix, version 2."  This will make it easier for
email clients to thread things correctly, and for human readers to know
they are looking at the most current revision of your set.

I think that's everything for now.  Additions, comments, suggestions are
welcome.

-- 

-Jay