[Libvirt-cim] [PATCH] [TEST] Add CodyingSystem and SubmittingPatches files

Guo Lian Yun yunguol at cn.ibm.com
Mon Jan 19 07:26:48 UTC 2009


libvirt-cim-bounces at redhat.com wrote on 2009-01-19 14:37:01:

> Very Good write up. I have few ideas along with some comments on this 
> patch(Please see inline for comments).
> We can improve it further including the information for "How to write a 
> test case?"
> This would include information like:
> 1) What kind of function should be included in the libraries present in 
> the XenKVMLib directory.
> 2) What should not be included in libraries present in the cimtest/lib.
> 3) What type of logger statements to be used for giving different 
> messages, for ex: use logger.info("Some information") or else use 
> logger.error("To print error")
> 4) Also, License information to be included in the test cases.
> 5) Including small description about what the test case does at the 
> beginning will help in maintenance in the long run.
> 6) Commands to submit a Single patch, commands to submit a Patch Set 
> using *"hg".
> T*his will make things easy for someone who is using *hg* for the first 
> time and might encourage more new people to contribute.
> We can rename the CodingStyle file to "Cimtest_Howto_Doc" and then 
> Include all the above information along with the CodingStyle in one 
file.
> We can advertise the presence of these documents at the 
> http://wiki.libvirt.org/page/Cimtest_todo page.
> Any thoughts ??
> 
  Good. When this patch is applied, I think we should check all the 
libraries and test cases 
  present to make them follow this coding style. Then we can cook up new 
tc.

> 
> Kaitlin Rupert wrote:
> > # HG changeset patch
> > # User Kaitlin Rupert <karupert at us.ibm.com>
> > # Date 1232145843 28800
> > # Node ID d94576542b4d6479ad04da466406c0cdb391f6e8
> > # Parent  684561f21975c7420cb7e15affc1eec4a8ed35ae
> > [TEST] Add CodyingSystem and SubmittingPatches files
> >
> > Signed-off-by: Kaitlin Rupert <karupert at us.ibm.com>
> >
> > +
> > +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, 
"Eliminateduplicate 
> > +           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 hereand 
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) 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.
> > 
> Need to include the step in case where hg export exposes something 
missing.
> Do we need to rework or just include the changes on top of the latest 
> changes commit the changes and send the two patches?
> OR include the new work in the latest cimtest tree, do the missing 
> changes and send all the changes as part of one patch.
> > +
> > +       (e) 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. 
> > +
> > +       (f) 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 ofthat 
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 (e) of the Single 
Patches 
> > +           guidelines to your "Patch [0 of 3]" header email, for 
> all the same 
> > +           reasons.
> > +
> > 
> Including the Sign-off in each cimtest patch is also very important.
> > +Questions/Comments on the Guidelines should be directed to:
> > + Kaitlin Rupert<kaitlin at linux.vnet.ibm.com>
> >
> > _______________________________________________
> > Libvirt-cim mailing list
> > Libvirt-cim at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvirt-cim
> > 
> 
> -- 
> Thanks and Regards,
> Deepti B. Kalakeri
> IBM Linux Technology Center
> deeptik at linux.vnet.ibm.com
> 
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20090119/586f3ec7/attachment.htm>


More information about the Libvirt-cim mailing list