qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 for-2.9 02/47] qapi: Make doc comments option


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v2 for-2.9 02/47] qapi: Make doc comments optional where we don't need them
Date: Wed, 15 Mar 2017 16:13:31 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 03/15/2017 07:56 AM, Markus Armbruster wrote:
>> Since we added the documentation generator in commit 3313b61, doc
>> comments are mandatory.  That's a very good idea for a schema that
>> needs to be documented, but has proven to be annoying for testing.
>> 
>> Make doc comments optional again, but add a new directive
>> 
>>     { 'pragma': { 'doc-required': true } }
>> 
>> to let a QAPI schema require them.
>> 
>> Add test cases for the new pragma directive.  While there, plug a
>> minor hole in includ directive test coverage.
>
> s/includ/include/

Will fix.

>> Require documentation in the schemas we actually want documented:
>> qapi-schema.json and qga/qapi-schema.json.
>> 
>> We could probably make qapi2texi.py cope with incomplete
>> documentation, but for now, simply make it refuse to run unless the
>> schema has 'doc-required': true.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
>> +=== Pragma directives ===
>> +
>> +Usage: { 'pragma': DICT }
>> +
>> +The pragma directive lets you control optional generator behavior.
>> +The dictionary's entries are pragma names and values.
>> +
>> +Pragma's scope is currently the complete schema.  Setting it to
>
> You can do:
>
> { 'pragma': { 'doc-required': true } }
> { 'pragma': { 'whitelist': [...] } }
>
> what you can't do is:
>
> { 'pragma': { 'doc-required': true } }
> { 'pragma': { 'doc-required': false } }
>
> Maybe s/Setting it/Setting a given pragma name/

Yes, that's better.  Perhaps "Setting the same pragma to ..."

>> +different values in parts of the schema doesn't work.

I considered rejecting all but the first pragma for any given pragma,
but decided just add this note for now.

>> +
>> +Pragma 'doc-required' takes a boolean value.  If true, documentation
>> +is required.  Default is false.
>> +
>> +
>
> The new tests are nice; thanks.
>
> Reviewed-by: Eric Blake <address@hidden>

Thanks!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]