libvirt-cim-bounces@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@us.ibm.com>
> > # Date 1232145843 28800
> > # Node ID d94576542b4d6479ad04da466406c0cdb391f6e8
> > # Parent  684561f21975c7420cb7e15affc1eec4a8ed35ae
> > [TEST] Add CodyingSystem and SubmittingPatches files
> >
> > Signed-off-by: Kaitlin Rupert <karupert@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@linux.vnet.ibm.com>
> >
> > _______________________________________________
> > Libvirt-cim mailing list
> > Libvirt-cim@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvirt-cim
> >  
>
> --
> Thanks and Regards,
> Deepti B. Kalakeri
> IBM Linux Technology Center
> deeptik@linux.vnet.ibm.com
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim@redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim