Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects

[Marc-André Lureau]

Hi

On Tue, Feb 6, 2018 at 11:12 AM, Markus Armbruster <address@hidden> wrote:
> Marc-André Lureau <address@hidden> writes:
>
>> Built-in objects remain unconditional.  Explicitly defined objects
>> use the condition specified in the schema.  Implicitly defined
>> objects inherit their condition from their users.  For most of them,
>> there is exactly one user, so the condition to use is obvious.  The
>> exception is the wrapped type's generated for simple union variants,
>> which can be shared by any number of simple unions.  The tight
>> condition would be the disjunction of the conditions of these simple
>> unions.  For now, use wrapped type's condition instead.  Much
>> simpler and good enough for now.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> Reviewed-by: Markus Armbruster <address@hidden>
>> ---
>>  scripts/qapi.py | 98 
>> ++++++++++++++++++++++++++++++++++++++-------------------
>>  1 file changed, 66 insertions(+), 32 deletions(-)
>>
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 27df0fcf48..8f54dead8d 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -991,8 +991,17 @@ def check_exprs(exprs):
>>  # Schema compiler frontend
>>  #
>>
>> +def listify_cond(ifcond):
>> +    if not ifcond:
>> +        return []
>> +    elif not isinstance(ifcond, list):
>> +        return [ifcond]
>> +    else:
>> +        return ifcond
>
> pylint complains:
>
>     R:995, 4: Unnecessary "else" after "return" (no-else-return)
>
> Matter of taste.  Mine happens to agree with pylint's.  Suggest:
>
>    def listify_cond(ifcond):
>        if not ifcond:
>            return []
>        if not isinstance(ifcond, list):
>            return [ifcond]
>        return ifcond
>

There are so many errors with pylint, that I don't bother running it.
How did you notice? you run diff of error output between commits?

pycodestyle --diff is more convenient, and silent on this code.

Feel free to touch if you pick the patch.

>> +
>> +
>>  class QAPISchemaEntity(object):
>> -    def __init__(self, name, info, doc):
>> +    def __init__(self, name, info, doc, ifcond=None):
>>          assert isinstance(name, str)
>>          self.name = name
>>          # For explicitly defined entities, info points to the (explicit)
> [...]
>



-- 
Marc-André Lureau