Re: [PATCH v6 9/9] iotests: add pylintrc file

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On 3/4/20 2:22 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> Repeatable results. run `pylint iotests.py` and you should get a pass.
>> 
>> Start your sentences with a capital letter, please.
>> 
>
> The German complains about the capitalization, but not the sentence
> fragment.

Heh!

>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>  tests/qemu-iotests/pylintrc | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>  create mode 100644 tests/qemu-iotests/pylintrc
>>>
>>> diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
>>> new file mode 100644
>>> index 0000000000..feed506f75
>>> --- /dev/null
>>> +++ b/tests/qemu-iotests/pylintrc
>>> @@ -0,0 +1,20 @@
>>> +[MESSAGES CONTROL]
>>> +
>>> +# Disable the message, report, category or checker with the given id(s). You
>>> +# can either give multiple identifiers separated by comma (,) or put this
>>> +# option multiple times (only on the command line, not in the configuration
>>> +# file where it should appear only once). You can also use "--disable=all" to
>>> +# disable everything first and then reenable specific checks. For example, if
>>> +# you want to run only the similarities checker, you can use "--disable=all
>>> +# --enable=similarities". If you want to run only the classes checker, but have
>>> +# no Warning level messages displayed, use "--disable=all --enable=classes
>>> +# --disable=W".
>>> +disable=invalid-name,
>>> +        missing-docstring,
>>> +        line-too-long,
>>> +        too-many-lines,
>>> +        too-few-public-methods,
>>> +        too-many-arguments,
>>> +        too-many-locals,
>>> +        too-many-branches,
>>> +        too-many-public-methods,
>>> \ No newline at end of file
>> 
>> Add the newline, please.
>> 
>> German pejorative for the too-many- and too-few- warnings: "Müsli".
>> Implies it's for muesli-knitters / granola-crunchers indulging their
>> orthorexia.
>> 
>
> They are useful at times as they can suggest when you are going a bit
> overboard on "organically grown" design. For cleaning an existing
> codebase, it's more of a hindrance to the immediate goal of establishing
> a baseline.

Yes, gentle nudges to reconsider your code organization can be useful.
But when we run pylint with the goal of getting no output, even warnings
are much more than gentle nudges.

> (*cough* I try to adhere to them in my own freshly written code, and
> disable per-line when I've decided to veto the suggestion. Not
> appropriate for a codebase like ours. As Max reminds me, it's just tests
> -- don't make them too clever or pretty.)
>
> Regardless. It's not appropriate here and now.
>
>> missing-docstring is not advisable for libraries.  Feels okay here.
>> 
>
> Ideally we do start using them, but it's out of scope here. Since I did
> some cleanup, I wanted to establish the baseline of what I adhered to.
>
> *not* suggest that it's the destination state.
>
> Adding proper docstrings should be done during mypy conversion once the
> types are determined, understood, and enforced. Not before then.
>
>> line-too-long might be worth cleaning up.  How many of them do we have
>> now?
>> 
>
> Five in iotests.py using the default length of 100. 15 if I limit to 80.
>
> If we agree that 100 is okay, I can tackle this in an amendment patch.
> If 80 is okay, I'm going to put it off as one coat of paint too many.
>
> (I will try to clean up the 100+ lines for my next version. I am
> hesitant to make a deeper cut because I have the feeling it's the type
> of series that will incur a lot of nitpicks on indent style.)

One step at a time.