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.
# 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
Kaitlin Rupert wrote: 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.
+ 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
Be 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.
+ + (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
+ 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
+ your patch on top of it. You should still indicate
+ 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
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. list as long tree. Now write the dependency 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