[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 13/28] qapi: Add some expr tests
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 13/28] qapi: Add some expr tests |
Date: |
Sun, 29 Mar 2015 10:27:12 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 03/27/2015 06:38 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> On 03/26/2015 09:55 AM, Markus Armbruster wrote:
>>>> Eric Blake <address@hidden> 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 '_').
Sounds good!
- [Qemu-devel] [PATCH v5 09/28] qapi: Prepare for catching more semantic parse errors, (continued)
[Qemu-devel] [PATCH v5 18/28] qapi: Unify type bypass and add tests, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 11/28] qapi: Rename anonymous union type in test, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 17/28] qapi: Allow true, false and null in schema json, Eric Blake, 2015/03/24