qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH] split c and cxx extra flags


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH] split c and cxx extra flags
Date: Tue, 6 Jun 2017 13:38:11 +0100

On Tue, Jun 6, 2017 at 1:20 PM, Bruno Dominguez <address@hidden> wrote:
> sending again the patch with the correct format and:
>
> 1. Matching --extra-cxxflags description with --cxx
> 2. Removing some extra spaces indicated by checkpatch script.

The code looks fine to me:
Reviewed-by: Stefan Hajnoczi <address@hidden>

A few more points about patch submission:

All email body text above '---' is the commit description that is
included in the git log.  Therefore the commit description should only
contain information that is useful in the version history, like the
rationale for this commit.

Please put changelog information below '---' so it does not become
part of the git log.  Once the patch has been merged it's not useful
to know that you're "sending again the patch with the correct format
and:" and so on.

The submission guidelines page says "send each new revision as a new
top-level thread, rather than burying it in-reply-to an earlier
revision, as many reviewers are not looking inside deep threads for
new patches".

There are tools that make it easy to send and manage patch series.
That way you don't have to invoke git-format-patch/git-send-email and
perform manual steps.  The tool I use is git-publish:
https://github.com/stefanha/git-publish

Putting this all together, please send a new top-level email like this:

Subject: [PATCH v2] configure: split c and cxx extra flags

There was no possibility to add specific cxx flags using the configure
file. So A new entrance has been created to support it.

Duplication of information in configure and rules.mak. Taking
QEMU_CFLAGS and add them to QEMU_CXXFLAGS, now the value of
QEMU_CXXFLAGS is stored in config-host.mak, so there is no need for
it.

The makefile for libvixl was adding flags for QEMU_CXXFLAGS in
QEMU_CFLAGS because of the addition in rules.mak. That was removed, so
adding them where it should be.

Signed-off-by: Bruno Dominguez <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
---
v2:
 * Matching --extra-cxxflags description with --cxx
 * Removing some extra spaces indicated by checkpatch script
 * Fixed patch format

...



reply via email to

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