[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 04/16] qapi: Clean up qapi.py per pep8
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v6 04/16] qapi: Clean up qapi.py per pep8 |
Date: |
Tue, 29 Sep 2015 07:00:37 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 09/29/2015 04:58 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> 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 <address@hidden>
> [...]
>> 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
signature.asc
Description: OpenPGP digital signature