Re: [Qemu-devel] [PATCH v5 13/28] qapi: Add some expr tests

[Eric Blake <eblake@redhat.com>]

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

On 03/27/2015 06:38 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 03/26/2015 09:55 AM, Markus Armbruster wrote:
>>> Eric Blake <eblake@redhat.com> writes:
>>>
>>>> Demonstrate that the qapi generator doesn't deal well with
>>>> expressions that aren't up to par. Later patches will improve
>>>> the expected results as the generator is made stricter.  Only
>>>> one of the added tests actually behaves sanely at rejecting
>>>> obvious problems.
>>>>
>>>
>>> qapi-code-gen.txt documents the naming conventions:
>>>
>>>     Types, commands, and events share a common namespace.  Therefore,
>>>     generally speaking, type definitions should always use CamelCase for
>>>     user-defined type names, while built-in types are lowercase. Type
>>>     definitions should not end in 'Kind', as this namespace is used for
>>>     creating implicit C enums for visiting union types.  Command names,
>>>     and field names within a type, should be all lower case with words
>>>     separated by a hyphen.  However, some existing older commands and
>>>     complex types use underscore; when extending such expressions,
>>>     consistency is preferred over blindly avoiding underscore.  Event
>>>     names should be ALL_CAPS with words separated by underscore.  The
>>>     special string '**' appears for some commands that manually perform
>>>     their own type checking rather than relying on the type-safe code
>>>     produced by the qapi code generators.
>>>
>>> We should either enforce the conventions consistently, or not at all.
>>>
>>> Enforcing them makes certain kinds of name clashes in generated C
>>> impossible.  If we don't enforce them, we should catch the clashes.
>>>
>>> Since I haven't read to the end of your series, I have to ask: do you
>>> intend to enforce them?
>>
>> I added tests to enforce it for event names, but did not enforce things
>> for command names or complex type members.  I guess that can be added on
>> top, if desired.
>>
>> So, did this patch get R-b?
> 
> I'd rather not enforce naming conventions just for events.
> 
> If we want to enforce them, let's do it consistently, and in a separate
> series that includes this patch.  Okay?

Sounds like I need a v6 respin then, where I drop my patch that attempts
to enforce all-caps event naming but did not enforce type or command
naming; but I will keep everything else (enforcing that names are valid
C identifiers + '-' and '.' (which both get flattened to '_').

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