On 05/11/2014 09:08 AM, Julio Faracco wrote:

When sending a series, it helps to use 'git send-email --cover-letter'
(or 'git format-patch --cover-letter' before regular 'git send-email')
to generate the 0/N lead-in letter.  Mail client threading is easier
when all patches are in-reply to the 0/N cover letter than when patches
2-N are in-reply to patch 1.  For example, my client sorts all replies
to the same message in the order received.  Look what happens with a
cover letter:

0/4
+ 1/4
| + Re: 1/4
+ 2/4
| + Re: 2/4
+ 3/4
| + Re: 3/4
+ 3/4
  + Re: 3/4

vs. how hard it is to associate 1/4 with its direct review reply without
a cover letter:

1/4
+ 2/4
| + Re: 2/4
+ 3/4
| + Re: 3/4
+ 4/4
| + Re: 4/4
+ Re: 1/4


> In "src/conf/" there are many enumeration (enum) declarations. Similar to the 
> recent cleanup to "src/util" directory, it's better to use a typedef for 
> variable types, function types and other usages. Other enumeration and 
> folders will be changed to typedef's in the future. Most of the files changed 
> in this commit are reltaed to Network (node_device_conf.h and 
> nwfilter_params.*) enums.
> 
> Signed-off-by: Julio Faracco <jcfara...@gmail.com>
> Signed-off-by: Eric Blake <ebl...@redhat.com>

Please don't add Signed-off-by: for anyone that did not actively write
(part of) the patch or tweak the commit as part of pushing it into an
upstream tree.  I add Signed-off-by when applying your commits to the
tree, to state that I created a new commit hash (even if all I do is
'git am' your patch, that is a different commit id in my tree than in
yours, so I have modified things).  But until I'm happy enough with the
patch to push it upstream, I haven't actually signed off the contents of
the patch; at this point in the game, you are sending the patch where
only you are responsible for what it looks like.  It's okay to mention
me in the commit message if I gave you ideas used in the patch, but do
that with something like "Suggested by:" rather than "Signed-off-by:".
Generally you don't see a second S-o-b for anything that is still
pending upstream inclusion unless it was truly a joint authorship situation.

[By the way, on this list we are fairly lax about the meaning of S-o-b,
or even its omission; although we are gradually morphing more towards
the model used by the Linux kernel.  On that list, they are absolute
sticklers, because they use S-o-b as a legal statement of blame for
copyright purposes, while we aren't there yet]

> ---
>  src/conf/node_device_conf.h |   24 ++++++++++++------------
>  src/conf/nwfilter_params.c  |    2 +-
>  src/conf/nwfilter_params.h  |   14 +++++++-------
>  3 files changed, 20 insertions(+), 20 deletions(-)

So, in spite of all that, your patch itself is fine, so I'm going to
amend the commit message to avoid long lines (hey - that means I
actually added my S-o-b via my normal policy after all) and push it shortly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to