qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH v2]util:Removed header qemu-common.h from path


From: Eric Blake
Subject: Re: [Qemu-trivial] [PATCH v2]util:Removed header qemu-common.h from path.c
Date: Fri, 3 Mar 2017 15:07:47 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0

On 03/03/2017 02:22 PM, Suramya Shah wrote:
> Signed-off-by: Suramya Shah <address@hidden>

Technically, this is v3 now (and your next submission will be v4).
Also, if you use 'git send-email -v4' to send your patch, it would
automatically put a space after the closing ] of the subject prefix (I
think 'git am' does the right thing even without the space, but when
most submissions look consistent in the archives, the ones that are
different stand out)

In the subject, put a space after the colon.

It's also fine that your subject says 'what' was done, but there is no
'why'; that's what the commit body is good for.  It's also good to write
patches in the imperative sense (present tense command form) rather than
past tense.  Visualize it as "this patch does something" (or even "apply
this patch for these results"), not "I wrote this patch to do something"
or "this patch did something".

Another tip: if you can easily identify a patch where a particular
situation started that you are now fixing, calling it out may help
reviewers.  In this case, looking at 'git log util/path.c', it looks
like commit f348b6d was the one that got rid of the need for using
qemu-common.h, when it moved declarations into other headers.  Showing
that you've researched the history gives more weight to your patch being
valid.

> --- fix of typo in v1 that broke compilation

'git am' requires '---' to be on a line of its own.  You want the
changelog description to occur on its own line, after the --- separator.

Also, since I gave a Reviewed-by: on your last mail, you can add it here
to mark that the patch body is unchanged since it was last reviewed.

Since this appears to be your first contribution, the maintainer that
incorporates your patch can probably clean up these (minor) mistakes, so
you may not need to resubmit (on the other hand, resubmitting makes it
less work for the maintainer, so feel free to do it if you'd like). If
you'd like more tips for successful submissions, there's a lot of
resources at http://wiki.qemu-project.org/Contribute/SubmitAPatch

Putting this all together, I suggest the following header if you want to
resubmit as a v4:


util: Remove unneeded header from path.c

qemu-common.h is no longer necessary, as of the refactoring done in
commit f348b6d.

Signed-off-by:...
Reviewed-by: Eric Blake <address@hidden>

---
v4: rewrite commit log based on Eric's hints

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]