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