Re: [Qemu-devel] [PATCHv51/3] qapi: Use an explicit input file

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2493 bytes --]

On 03/30/2014 02:35 PM, Lluís Vilanova wrote:

Missing a space in the subject line (this is not version 51 of the
patches,  but v5 and patch 1/3 :)

> Use an explicit input file on the command-line instead of reading from standard input
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---

>  25 files changed, 72 insertions(+), 41 deletions(-)

Now we've got competing versions of the patches between Benoît and Lluís
(and apologies to either of you if I accidentally mis-spell the 'i' in
your name due to my keyboard not making it as easy to add accents).

> 
> diff --git a/Makefile b/Makefile
> index ec74039..c7cec01 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -237,23 +237,35 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
>  
>  qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
>  $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> -	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, "  GEN   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
> +		$(gen-out-type) -i "$<" -o qga/qapi-generated -p "qga-", \
> +		"  GEN   $@")

I _like_ that you used \ to break long lines to fit into 80 columns.
But doing it at the same time as adding an option makes it a little
harder to see the addition; maybe it's worth splitting into two patches
(one for the reformatting but no semantic change, the other for the
addition)?  And maybe by using '-i $<' at the place where '< $<'
currently appears will make it a bit easier to see the change being
done, rather than having to spot that things are reordered to provide -i
earlier?

> +++ b/docs/qapi-code-gen.txt
> @@ -221,7 +221,7 @@ created code.
>  Example:
>  
>      mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \
> -      --output-dir="qapi-generated" --prefix="example-" < example-schema.json
> +      --input-file=example-schema.json --output-dir="qapi-generated" --prefix="example-"

On Benoît's version, the comment was made that it is more
'compiler-like' to take the input file as an argument name, rather than
an option name (that is, use '$<' directly, rather than '-i $<' in the
Makefile) - but if we do that, it is best to put the argument after all
options.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]