Re: [Qemu-devel] [PATCH v6 04/16] qapi: Clean up qapi.py per pep8

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2528 bytes --]

On 09/29/2015 04:58 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Silence pep8, and make pylint a bit happier.  Just style cleanups,
>> plus killing a useless comment in camel_to_upper(); no semantic
>> changes.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> [...]
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index a2d869e..7e8a99d 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
> [...]
>> @@ -667,22 +695,23 @@ def check_keys(expr_elem, meta, required, optional=[]):
>>      if not isinstance(name, str):
>>          raise QAPIExprError(info,
>>                              "'%s' key must have a string value" % meta)
>> -    required = required + [ meta ]
>> +    required = required + [meta]
>>      for (key, value) in expr.items():
>> -        if not key in required and not key in optional:
>> +        if key not in required + optional:
> 
> The purely mechanical cleanup would be
> 
>         if key not in required and key not in optional:
> 
> Yours is a bit more concise, but I'm not sure it's actually easier to
> read.  It's also less efficient unless a sufficiently smart compiler
> transforms it back.  Not that efficiency matters here.

The purely mechanical version still fits the line, so I'm not strongly
attached to the concise version.


>> @@ -1307,14 +1338,14 @@ def camel_to_upper(value):
>>          c = c_fun_str[i]
>>          # When c is upper and no "_" appears before, do more checks
>>          if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
>> -            # Case 1: next string is lower
>> -            # Case 2: previous string is digit
>> -            if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
>> -            c_fun_str[i - 1].isdigit():
>> +            if i < (l - 1) and c_fun_str[i + 1].islower():
> 
> I doubt we need the parenthesis around (l - 1).

Probably not; I was just preserving the existing text.

> 
>> +                new_name += '_'
>> +            elif c_fun_str[i - 1].isdigit():
>>                  new_name += '_'
>>          new_name += c
>>      return new_name.lstrip('_').upper()
>>
>> +
>>  def c_enum_const(type_name, const_name, prefix=None):
>>      if prefix is not None:
>>          type_name = prefix
> [...]
> 
> Happy to touch up on commit to my tree.

Or, since you want a respin of 5, I can catch them.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]