[PATCH] Patch guidelines added to SubmittingPatches
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1196350280 18000 # Node ID a4193d586c20dd4aa9767f1299d9a1e91c0786a8 # Parent c478a5b30689a80159588c8f914ac97263694372 Patch guidelines added to SubmittingPatches. I did a bit of editing from the original suggestions, so any grammar-minded people should probably run through it once. I also incorporated the following from the team call: The two "stay on track"-style rules were merged. Patch numbering for resends was expanded to all patches, not just patchsets. Including an example test for edge cases was added. Patch dependency handling was added. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r c478a5b30689 -r a4193d586c20 doc/SubmittingPatches --- a/doc/SubmittingPatches Wed Nov 28 11:06:22 2007 -0800 +++ b/doc/SubmittingPatches Thu Nov 29 10:31:20 2007 -0500 @@ -33,3 +33,91 @@ using your real name (sorry, no pseudony using your real name (sorry, no pseudonyms or anonymous contributions.) + +Guidelines for Submitting Patches. + + Patches should be submitted using Mercurial's patchbomb extension, + and we recommend using the queues extension as well. On top of that, + we have some guidelines you should follow when submitting patches. + This makes reviewing patches easier, which in turns improves the + chances of your patch being accepted in a timely fashion. + +Single Patches: + + (a) 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. Avoid just saying something like, "Various fixes to + AllocationCapabilities." Add a list of what was actually fixed, + like, "Add EnumInstanceNames support," and, "Eliminate duplicate + instances." + + (b) The first line of your commit message will be the subject of the + patch email when you send it out, so write it like a subject. Keep + it short and to the point, then start a new line and feel free to be + as verbose as you need to be. The entire commit message will be + included in the patch. + + (c) 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. The same goes for + nitpicking. While it might be only a line or two here and there + that is off track, this is actually one of the easiest ways to make + a patch difficult to review. All the trivial changes hide what is + really going on. Make the unrelated changes a new patch or don't + make them at all. + + (d) If your patch addresses a strange issue or a rare edge case that + the reviewers are unlikely to be familiar with, make sure the commit + message include some example testcase with results, so the reviewers + can verify your patch more quickly. + + (e) Before you email, export. If you have a patch called "alloc_fixes", + which would be emailed with "hg email alloc_fixes", you should first + run "hg export alloc_fixes". This lets you 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? + This is the single biggest step in ensuring you have a good patch. + + (f) If your patch needs to be reworked and resent, prepend a "version + number" to the first line of the commit message. For example, "Add + EnumInstance to RASD," becomes "#2 Add EnumInstance to RASD." This + helps mail readers thread discussions correctly and helps + maintainers know they are applying the right version of your patch. + At the end of the commit message, explain what is different from one + version to the next. Nobody likes having to diff a diff. + + (g) If your patch depends on a patch that exists on the mailing list but + not in the tree, it is okay to send your patch to the list as long + as your commit message mentions the dependency. It is also a good + idea to import the patch into your tree before you make your patch. + For example, a new patch called "cu_statusf API change" is on the + list, and your patch needs the new API. Save the email (no need to + trim headers) as api_change.eml, then do "hg qimport api_change.eml" + and "hg qpush" so that the patch is applied to your tree. Now write + your patch on top of it. You should still indicate the dependency + in your commit message. + + +Patchsets: + + (a) 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. + + (b) 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." + + (c) If you resend a patchset, apply part (f) of the Single Patches + guidelines to your "Patch [0 of 3]" header email, for all the same + reasons. + +Questions/Comments on the Guidelines should be directed to: +Jay Gagnon <grendel@linux.vnet.ibm.com> +Patch Compliance Officer \ No newline at end of file
JG> # HG changeset patch JG> # User Jay Gagnon <grendel@linux.vnet.ibm.com> JG> # Date 1196350280 18000 JG> # Node ID a4193d586c20dd4aa9767f1299d9a1e91c0786a8 JG> # Parent c478a5b30689a80159588c8f914ac97263694372 JG> Patch guidelines added to SubmittingPatches. Looks good to me. I'll let it fester on the list for comments for a bit. Did you want to add a page to the web content for this as well? Either that or a pointer on one of the existing pages to this file might be a good idea. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> # HG changeset patch JG> # User Jay Gagnon <grendel@linux.vnet.ibm.com> JG> # Date 1196350280 18000 JG> # Node ID a4193d586c20dd4aa9767f1299d9a1e91c0786a8 JG> # Parent c478a5b30689a80159588c8f914ac97263694372 JG> Patch guidelines added to SubmittingPatches.
Looks good to me. I'll let it fester on the list for comments for a bit.
Did you want to add a page to the web content for this as well? Either that or a pointer on one of the existing pages to this file might be a good idea.
I wouldn't mind adding a page to the site, especially since then I could use some nice formatting to make it a little easier to read. I'm unfamiliar with this xsl thing though, so a primer on that would be appreciated. -- -Jay
JG> I wouldn't mind adding a page to the site, especially since then I JG> could use some nice formatting to make it a little easier to read. JG> I'm unfamiliar with this xsl thing though, so a primer on that JG> would be appreciated. If you look at the top of the site.xsl, you see all the external pages that will be generated. Adding something like this would be your first step. <xsl:when test="$name = '#Patches'"> <xsl:text>patches.html</xsl:text> </xsl:when> Then, in the libvirt-cim.html, add a new <h2><a name="Patches"> and the content between that and the next <h2> (or EOF) will go into patches.html. Just run "make" in doc/ to regenerate it each time you tweak it. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> I wouldn't mind adding a page to the site, especially since then I JG> could use some nice formatting to make it a little easier to read. JG> I'm unfamiliar with this xsl thing though, so a primer on that JG> would be appreciated.
If you look at the top of the site.xsl, you see all the external pages that will be generated. Adding something like this would be your first step.
<xsl:when test="$name = '#Patches'"> <xsl:text>patches.html</xsl:text> </xsl:when>
Then, in the libvirt-cim.html, add a new <h2><a name="Patches"> and the content between that and the next <h2> (or EOF) will go into patches.html. Just run "make" in doc/ to regenerate it each time you tweak it.
Just curious, in site.xsl, why do some of the test conditions have truncated names? For example: <xsl:when test="$name = '#Documentat'"> <xsl:text>docs.html</xsl:text> </xsl:when> It seems that anytime $name would end in "ion" it gets the "ion" or the "on" part at the end cut off. -- -Jay
On Mon, Dec 03, 2007 at 11:44:41AM -0500, Jay Gagnon wrote:
Dan Smith wrote:
JG> I wouldn't mind adding a page to the site, especially since then I JG> could use some nice formatting to make it a little easier to read. JG> I'm unfamiliar with this xsl thing though, so a primer on that JG> would be appreciated.
If you look at the top of the site.xsl, you see all the external pages that will be generated. Adding something like this would be your first step.
<xsl:when test="$name = '#Patches'"> <xsl:text>patches.html</xsl:text> </xsl:when>
Then, in the libvirt-cim.html, add a new <h2><a name="Patches"> and the content between that and the next <h2> (or EOF) will go into patches.html. Just run "make" in doc/ to regenerate it each time you tweak it.
Just curious, in site.xsl, why do some of the test conditions have truncated names? For example:
<xsl:when test="$name = '#Documentat'"> <xsl:text>docs.html</xsl:text> </xsl:when>
It seems that anytime $name would end in "ion" it gets the "ion" or the "on" part at the end cut off.
Haha, it's the magic of reuse (Dan started of will libvirt html I think) and the fact that i used Amaya a long time ago to edit such files, and it was generating the IDs that way. Really no big deal or significance, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard wrote:
On Mon, Dec 03, 2007 at 11:44:41AM -0500, Jay Gagnon wrote:
Dan Smith wrote:
JG> I wouldn't mind adding a page to the site, especially since then I JG> could use some nice formatting to make it a little easier to read. JG> I'm unfamiliar with this xsl thing though, so a primer on that JG> would be appreciated.
If you look at the top of the site.xsl, you see all the external pages that will be generated. Adding something like this would be your first step.
<xsl:when test="$name = '#Patches'"> <xsl:text>patches.html</xsl:text> </xsl:when>
Then, in the libvirt-cim.html, add a new <h2><a name="Patches"> and the content between that and the next <h2> (or EOF) will go into patches.html. Just run "make" in doc/ to regenerate it each time you tweak it.
Just curious, in site.xsl, why do some of the test conditions have truncated names? For example:
<xsl:when test="$name = '#Documentat'"> <xsl:text>docs.html</xsl:text> </xsl:when>
It seems that anytime $name would end in "ion" it gets the "ion" or the "on" part at the end cut off.
Haha, it's the magic of reuse (Dan started of will libvirt html I think) and the fact that i used Amaya a long time ago to edit such files, and it was generating the IDs that way. Really no big deal or significance,
Oh okay. I was hoping it was something like that, really. I was slightly worried about what the possible explanations could be. :) -- -Jay
participants (3)
-
Dan Smith -
Daniel Veillard -
Jay Gagnon