Re: [PATCH 4/4] qapi: Brush off some (py)lint

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On 3/4/20 3:01 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> On 2/27/20 9:45 AM, Markus Armbruster wrote:
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>
>>> I wrote some pylint cleanup for iotests recently, too. Are you targeting
>>> a subset of pylint errors to clean here?
>>>
>>> (Do any files pass 100%?)
>> 
>> Surely you're joking, Mr. Snow!
>> 
>> I'm chipping away at pylint's gripes.  I ran it with the following
>> messages disabled:
>> 
>>     bad-whitespace,
>>     fixme,
>>     invalid-name,
>>     missing-docstring,
>>     too-few-public-methods,
>>     too-many-arguments,
>>     too-many-branches,
>>     too-many-instance-attributes,
>>     too-many-lines,
>>     too-many-locals,
>>     too-many-statements,
>>     unused-argument,
>>     unused-wildcard-import,
>> 
>> These are not all obviously useless.  They're just not what I want to
>> focus on right now.
>> 
>
> Yes, understood - so my approach is disable what I don't intend to fix,
> commit the pylintrc to prevent backslide, and move on.
>
> I think we have a difference in what a pylintrc means to us (the goal
> vs. the current status.)
>
> I didn't mean "100% without caveats", just "100% in some subset of checks".
>
> (I assume the answer is still no.)

To turn the answer into a yes, I'd have to disable the messages below,
and some of them I'd rather keep.

Tacking

    # pylint: disable=...

to existing troublemakers may or may not be worth the ugliness (it needs
to go on the same line, which almost invariably makes it awkwardly long).

>> Remaining:
>> 
>> 1 x C0330: Wrong continued indentation (remove 19 spaces).
>> 
>>     Accident, will fix in v2.
>> 
>> 8 x R0201: Method could be a function (no-self-use)
>> 
>>     Yes, but the override in a sub-class does use self.
>> 
>> 2 x W0212: Access to a protected member _body of a client class (protected-access)
>> 
>>     Needs cleanup, but not now.
>> 
>> 6 x W0401: Wildcard import qapi.common (wildcard-import)
>> 
>>     Not sure I care.  I'd prefer not to have more wildcard imports,
>>     though.
>> 
>> 2 x W0603: Using the global statement (global-statement)
>> 
>>     Cleanup is non-trivial.  Not now.
>> 
>> I also ran pycodestyle-3:
>> 
>> 1 x E127 continuation line over-indented for visual indent
>> 
>>     Same as pylint's C0330, will fix in v2.
>> 
>> 3 x E261 at least two spaces before inline comment
>> 
>>     I blame Emacs.  Left for another day.
>> 
>> 8 x E501 line too long
>> 
>>     Left for another day.
>> 
>> 1 x E713 test for membership should be 'not in'
>> 
>>     I missed that one, will fix in v2.
>> 
>>> Consider checking in a pylintrc file that lets others run the same
>>> subset of pylint tests as you are doing so that we can prevent future
>>> regressions.
>> 
>> Working towards it, slowly.
>> 
>>> Take a peek at [PATCH v6 0/9] iotests: use python logging​
>>>
>>> Thanks for this series. I had a very similar series sitting waiting to
>>> go out, but this goes further in a few places.
>> 
>> Thanks!
>>