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

[John Snow]

On 3/4/20 2:22 AM, Markus Armbruster wrote:
> John Snow <address@hidden> 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.

>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>>  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.

(*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.)

--js