Re: [Qemu-devel] [PATCH v8 09/26] qapi: De-duplicate enum code generation

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 09/16/2015 05:06 AM, Markus Armbruster wrote:
>> Duplicated in commit 21cd70d.  Yes, we can't import qapi-types, but
>> that's no excuse.  Move the helpers from qapi-types.py to qapi.py, and
>> replace the duplicates in qapi-event.py.
>> 
>> The generated event enumeration type's lookup table becomes
>> const-correct (see commit 2e4450f), and uses explicit indexes instead
>> of relying on order (see commit 912ae9c).
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  docs/qapi-code-gen.txt |  9 ++++---
>>  scripts/qapi-event.py  | 67 
>> +++-----------------------------------------------
>>  scripts/qapi-types.py  | 55 -----------------------------------------
>>  scripts/qapi.py        | 55 +++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 64 insertions(+), 122 deletions(-)
>> 
>
>> +++ b/scripts/qapi.py
>> @@ -1497,6 +1497,61 @@ def guardend(name):
>>  ''',
>>                   name=guardname(name))
>>  
>> +def generate_enum_lookup(name, values, prefix=None):
>
> To keep pep8 happier, you could use two blank lines before def here...
>
>> +    return ret
>> +
>> +def generate_enum(name, values, prefix=None):
>
> and here.  Then again, 13/26 does more of these sorts of cleanups, and
> v7 had the same use of 1 blank line.  Up to you if it is worth avoiding
> the churn; but it is whitespace either way so it doesn't affect review.

Matches the code around it, which trumps pep8.

pep8 is quite unhappy with qapi.py.  This series makes it slightly
happier by deleting unhappy code.  Finishing the conversion to the new
intermediate representation involves folding semantic analysis into
QAPISchema, good opportunity to tidy up the moved code.  After that, we
should probably tidy up whatever's left.

> Also, do you still need prefix=None, or can we rely on the fact that now
> all callers supply prefix by virtue of the visitor callback, and make
> the parameter non-optional?

There are calls without prefix left in qapi-event.py.

> Reviewed-by: Eric Blake <address@hidden>

Thanks!