Re: [PATCH 06/37] qapi: delint using flake8

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On 9/16/20 8:12 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> Petty style guide fixes and line length enforcement.  Not a big win, not
>>> a big loss, but flake8 passes 100% on the qapi module, which gives us an
>>> easy baseline to enforce hereafter.
>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>   scripts/qapi/.flake8     |  2 ++
>>>   scripts/qapi/commands.py |  3 ++-
>>>   scripts/qapi/schema.py   |  8 +++++---
>>>   scripts/qapi/visit.py    | 15 ++++++++++-----
>>>   4 files changed, 19 insertions(+), 9 deletions(-)
>>>   create mode 100644 scripts/qapi/.flake8
>>>
>>> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
>>> new file mode 100644
>>> index 0000000000..45d8146f3f
>>> --- /dev/null
>>> +++ b/scripts/qapi/.flake8
>>> @@ -0,0 +1,2 @@
>>> +[flake8]
>>> +extend-ignore = E722  # Pylint handles this, but smarter.
>> I guess you mean pylint's W0702 a.k.a. bare-except.  What's wrong
>> with
>> flake8's E722 compared to pylint's W0702?
>> 
>
> Flake8 will warn on *any* bare except, but Pylint's is context-aware
> and will suppress the warning if you re-raise the exception.

Should this information be worked into the comment?

> I don't actually think this comes up in the qapi code base, but it
> does come up in the ./python/qemu code base.
>
> (One of my goals is unifying the lint checking regime for both packages.)
>
>>> \ No newline at end of file
>> So put one there :)
>> 
>
> Whupps, okay.
>
>>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
>>> index e1df0e341f..2e4b4de0fa 100644
>>> --- a/scripts/qapi/commands.py
>>> +++ b/scripts/qapi/commands.py
>>> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):
>>>   def gen_marshal_output(ret_type):
>>>       return mcgen('''
>>>   -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,
>>> QObject **ret_out, Error **errp)
>>> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
>>> +                                          Error **errp)
>> The continued parameter list may become misalignd in generated C.
>> E.g.
>>      static void qmp_marshal_output_BlockInfoList(BlockInfoList
>> *ret_in, QObject **ret_out,
>>                                                Error **errp)
>>      {
>>      ...
>>      }
>> Do we care?
>> 
>
> Yeah, I don't know. Do we?

I care, but I also care for automated style checks.

> It actually seemed more annoying to try and get flake8 to make an
> exception for these handful of examples.
>
> Path of least resistance led me here, but I can try and appease both
> systems if you'd prefer.

Up to now, I ran the style checkers manually, and this was just one of
several complaints to ignore, so I left the code alone.

If it gets in the way of running them automatically, and messing up the
generated code slightly is the easiest way to get it out of the way,
then I can accept the slight mess.

>> More of the same below.
>> 
>>>   {
>>>       Visitor *v;
>>>   
>> [...]
>>