All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members
Date: Wed, 2 Dec 2015 12:43:26 -0700	[thread overview]
Message-ID: <565F49DE.8060303@redhat.com> (raw)
In-Reply-To: <87oae8ix9b.fsf@blackfin.pond.sub.org>

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

On 12/02/2015 10:19 AM, Markus Armbruster wrote:

>> With your patch, the positive tests no longer work in isolation.  You
>> were getting lucky that things sorted such that 'Foo' was checked for
>> correctness prior to 'UuidInfo'; but if you comment out the 'Foo'
>> declaration, or rename from 'Foo' to something else that hashes after
>> 'UuidInfo', then args-member-case and union-branch-case start reporting
>> failures about UuidInfo (and only enum-member-case honors the
>> whitelist).
> 
> You're right.
> 

>> +++ w/tests/qapi-schema/args-member-case.json
>> @@ -1,3 +1,3 @@
>>  # Member names should be 'lower-case' unless the struct/command is
>> whitelisted
>> -{ 'command': 'UuidInfo', 'data': { 'Arg': 'int' } }
>> +{ 'command': 'x-UuidInfo', 'data': { 'Arg': 'int' } }
> 
> Will fail as soon as we enforce the command naming convention.  To avoid
> that, we need to add a suitable name to the whitelist.
> 
> However, we don't actually have any command parameters to whitelist.
> Why bother testing it then?

Good point.


> 
>>From 44f07a40c8b9b5d1f24833028b5dacde1fd50c80 Mon Sep 17 00:00:00 2001
> From: Markus Armbruster <armbru@redhat.com>
> Date: Wed, 2 Dec 2015 17:41:55 +0100
> Subject: [PATCH] fixup! qapi: Enforce (or whitelist) case conventions on qapi
>  members
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py                          | 6 ++----
>  tests/qapi-schema/args-member-case.err   | 2 +-
>  tests/qapi-schema/args-member-case.json  | 3 +--
>  tests/qapi-schema/enum-member-case.err   | 2 +-
>  tests/qapi-schema/enum-member-case.json  | 4 ++--
>  tests/qapi-schema/union-branch-case.err  | 2 +-
>  tests/qapi-schema/union-branch-case.json | 3 +--
>  7 files changed, 9 insertions(+), 13 deletions(-)

Looks good.  Commit 6c1eb345 on your qapi-not-next branch has this
already squashed in, but lacking your S-o-b.  But that only affects the
commit message, so I'll go ahead and base my future patch submissions on
top of the current contents of qapi-not-next (it will be interesting to
see how many patches land from various trees right after 2.6 opens up...)

-- 
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 --]

  reply	other threads:[~2015-12-02 19:43 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-02  5:20 [Qemu-devel] [PATCH v14 00/15] qapi member collision (post-introspection cleanups, subset D) Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 01/15] qobject: Simplify QObject Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 02/15] qobject: Rename qtype_code to QType Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 03/15] qapi: Convert QType into QAPI built-in enum type Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 04/15] qapi: Simplify visiting of alternate types Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 05/15] qapi-types: Drop unnedeed ._fwdefn Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 06/15] qapi: Inline _make_implicit_tag() Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 07/15] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 08/15] qapi: Simplify visits of optional fields Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 09/15] qapi: Shorter " Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 10/15] qapi: Prepare new QAPISchemaMember base class Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 11/15] qapi: Track enum values by QAPISchemaMember, not string Eric Blake
2015-12-02 15:52   ` Markus Armbruster
2015-12-02 16:09     ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 12/15] qapi: Populate info['name'] for each entity Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members Eric Blake
2015-12-02 11:51   ` Markus Armbruster
2015-12-02 13:41     ` Eric Blake
2015-12-02 15:19       ` Eric Blake
2015-12-02 17:19         ` Markus Armbruster
2015-12-02 19:43           ` Eric Blake [this message]
2015-12-02 20:27             ` Markus Armbruster
2015-12-02 16:48       ` Markus Armbruster
2015-12-02 14:11     ` Eric Blake
2015-12-02 15:30     ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 14/15] qapi: Move duplicate collision checks to schema check() Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 15/15] qapi: Detect base class loops Eric Blake
2015-12-02 17:57 ` [Qemu-devel] [PATCH v14 00/15] qapi member collision (post-introspection cleanups, subset D) Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=565F49DE.8060303@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.