From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38163) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTE5o-0003oA-Q8 for qemu-devel@nongnu.org; Thu, 27 Mar 2014 13:27:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTE5i-0005N7-CS for qemu-devel@nongnu.org; Thu, 27 Mar 2014 13:27:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64904) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTE5h-0005Mt-TE for qemu-devel@nongnu.org; Thu, 27 Mar 2014 13:27:46 -0400 Message-ID: <53345F8A.1040607@redhat.com> Date: Thu, 27 Mar 2014 11:27:38 -0600 From: Eric Blake MIME-Version: 1.0 References: <1395934399-18769-1-git-send-email-benoit.canet@irqsave.net> <1395934399-18769-3-git-send-email-benoit.canet@irqsave.net> In-Reply-To: <1395934399-18769-3-git-send-email-benoit.canet@irqsave.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Ategkf5Xa20Df5MMtuUvpXvWdUMIoGEIN" Subject: Re: [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , qemu-devel@nongnu.org Cc: anthony@codemonkey.ws, Benoit Canet , armbru@redhat.com, wenchaoqemu@gmail.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Ategkf5Xa20Df5MMtuUvpXvWdUMIoGEIN Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/27/2014 09:33 AM, Beno=C3=AEt Canet wrote: > This patch is here to pave the way for the JSON include directive which= > will need to do include loop detection. >=20 Would also be nice to mention that it improves the error message quality. While 3/3 is definitely 2.1 material, 1/3 and 2/3 could arguably be committed in 2.0 as bug fixes due to the improved errors (but it's a stretch - personally I'm fine with saving the whole series for 2.1, as the error messages are in the build chain only for developers to see, and not user-visible in the end product) > Signed-off-by: Benoit Canet > --- > 24 files changed, 76 insertions(+), 51 deletions(-) >=20 > diff --git a/Makefile b/Makefile > index ec74039..9bec4ff 100644 > --- a/Makefile > +++ b/Makefile > @@ -236,24 +236,24 @@ gen-out-type =3D $(subst .,-,$(suffix $@)) > qapi-py =3D $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddic= t.py > =20 > 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 $(q= api-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(ge= n-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SR= C_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qg= a-" < $<, " GEN $@") While I don't mind GNU getopt's ability to reorganize options to occur after non-options, I'm not sure it's the smartest thing to do here. Either the input file should be a new option (as in '-i input -o output') or you should consider ordering the command line to put the file name after all options ('-o output input' rather than 'input -o output'). For that matter, I feel that named options are always more flexible than positional arguments. That said, it looks like this script _already_ has a positional argument (gen-out-type) occurring before options, so you aren't making it any worse. Therefore, if you don't respin it to take the input file name as an option, I can live with: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Ategkf5Xa20Df5MMtuUvpXvWdUMIoGEIN Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTNF+KAAoJEKeha0olJ0Nqfu4H/j3khuX3vdFOS3gdm24NXvOX SSaW4W36JJo920ks8B6sr+YJEKtGAvlNFDI6UlpeLQuDs/p3+sIFATy7P9w+5Aao kYURr1wabSiOl66sIbVH8EtKkyQigf7Su66axWdWzVApL7axzu/4B6W6Dd+iYn5L 72FXUhpso7TurC8Wp6eMn1+OQxrKAmbCHSbjfmNdx+ZEpTv0HwjpP4Q4Kc4kNMS2 /fMa3mBodfFCOpFW9z5X8OPqX+3BbZyGyGcctrRNprRw7S7QZ4zr+vm0LkS4EawO omZrPYLIU+p41ielgATj3fvuDEjNg27ktz37+FntYmldhoPnez0P3Opq6DY5cbc= =FwAk -----END PGP SIGNATURE----- --Ategkf5Xa20Df5MMtuUvpXvWdUMIoGEIN--