[Libvirt-cim] Best practices for patchsets

Dan Smith danms at us.ibm.com
Tue Nov 27 18:17:20 UTC 2007


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

100% agree.  MQ allows you to edit a patch description at any time
with "hg qrefresh -e", so it should be easy to update the message with
information as you proceed.  For patches that do more than one
significant thing, a list of changes in the message would be good.

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

110% agree.  Patches are much easier to read if they're small.  Even
if you have two patches, one named "cleanups" and another named "fix
specific issue X", that's better than just lumping the cleanups in
with the fix.

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

Agree.

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

Yes, re-sending the whole set makes it much easier for me to apply the
relevant patches instead of cherry-picking the last-known-good version
of each patch of a set.  It also helps reviewers set context on a
specific change that is being iterated.

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

Agreed, especially with respect to the "human readers" part.  I don't
know that ", version 2" needs to be the exact suffix, but something
consistent and informative would be good.

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

Thanks a lot for doing this.  After we settle on some of these, can
you write this up as a patch to the SubmittingPatches file in the
tree?  That way we can claim it's policy :)

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms at us.ibm.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20071127/253b4c7b/attachment.sig>


More information about the Libvirt-cim mailing list