Re: [Qemu-devel] [PATCH for-2.9 34/47] qapi: Move empty doc section checking to doc parser

2017-03-15 Thread Markus Armbruster 
Eric Blake  writes:

> On 03/13/2017 01:23 AM, Markus Armbruster wrote:
>> Markus Armbruster  writes:
>> 
>>> Results in a more precise error location, but the real reason is
>>> emptying out check_docs() step by step.
>>>
>>> Signed-off-by: Markus Armbruster 
>> 
>> Perhaps we should simply drop this error condition.  Are empty sections
>> this a mistake users make accidentally?
>
> Parse error; did you mean "empty sections _like_ this"?

Yes.

> I'm okay with keeping the error; especially if we can't guarantee that
> the generator copes gracefully with an empty section (different than an
> omitted section).

We'd have to verify it does.

>On the other hand, even if we remove the error,
> you're probably right that anyone proposing a patch for incorporation
> that adds an empty section will have to explain themselves, whether or
> not the parser flagged it, and if the error is cheap to maintain in the
> parser, then it saves some review cycles.

The patch adds two methods and changes three existing ones just to catch
empty sections.  I can't help to ask: why bother?

Re: [Qemu-devel] [PATCH for-2.9 34/47] qapi: Move empty doc section checking to doc parser

2017-03-14 Thread Eric Blake 
On 03/13/2017 01:23 AM, Markus Armbruster wrote:
> Markus Armbruster  writes:
> 
>> Results in a more precise error location, but the real reason is
>> emptying out check_docs() step by step.
>>
>> Signed-off-by: Markus Armbruster 
> 
> Perhaps we should simply drop this error condition.  Are empty sections
> this a mistake users make accidentally?

Parse error; did you mean "empty sections _like_ this"?

I'm okay with keeping the error; especially if we can't guarantee that
the generator copes gracefully with an empty section (different than an
omitted section).  On the other hand, even if we remove the error,
you're probably right that anyone proposing a patch for incorporation
that adds an empty section will have to explain themselves, whether or
not the parser flagged it, and if the error is cheap to maintain in the
parser, then it saves some review cycles.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.9 34/47] qapi: Move empty doc section checking to doc parser

2017-03-14 Thread Eric Blake 
On 03/13/2017 01:18 AM, Markus Armbruster wrote:
> Results in a more precise error location, but the real reason is
> emptying out check_docs() step by step.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi.py | 20 ++--
>  tests/qapi-schema/doc-empty-section.err |  2 +-
>  2 files changed, 15 insertions(+), 7 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.9 34/47] qapi: Move empty doc section checking to doc parser

2017-03-13 Thread Markus Armbruster 
Markus Armbruster  writes:

> Results in a more precise error location, but the real reason is
> emptying out check_docs() step by step.
>
> Signed-off-by: Markus Armbruster 

Perhaps we should simply drop this error condition.  Are empty sections
this a mistake users make accidentally?