Re: [PATCH 07/37] qapi: add pylintrc

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 9/16/20 8:30 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> Add a skeleton pylintrc file. Right now, it ignores quite a few things.
>>> Files will be removed from the known-bad list throughout this and
>>> following series as they are repaired.
>>>
>>> Note: Normally, pylintrc would go in the folder above the module, but as
>>> that folder is shared by many things, it is going inside the module
>>> folder now.
>>>
>>> Due to some bugs in different versions of pylint (2.5.x), pylint does
>>> not correctly recognize when it is being run from "inside" a module, and
>>> must be run *outside* of the module.
>>>
>>> Therefore, to run it, you must:
>>>
>>>   > cd :/qemu/scripts
>> -bash: cd: :/qemu/scripts: No such file or directory
>> ;-P
>> 
>>>   > pylint qapi/ --rcfile=qapi/pylintrc
>> Why not
>>     $ pylint scripts/qapi --rcfile=scripts/qapi/pylintrc
>> 
>
> No reason I'm aware of, I have just been testing with CWD at the
> scripts dir myself because of how python imports work.
>
> If it works this way, enjoy!

It works, and it simplifies the commmit message.

>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>   scripts/qapi/pylintrc | 74 +++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 74 insertions(+)
>>>   create mode 100644 scripts/qapi/pylintrc
>>>
>>> diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
>>> new file mode 100644
>>> index 0000000000..c2bbb8e8e1
>>> --- /dev/null
>>> +++ b/scripts/qapi/pylintrc
>>> @@ -0,0 +1,74 @@
>>> +[MASTER]
>>> +
>>> +# Add files or directories matching the regex patterns to the blacklist. 
>>> The
>>> +# regex matches against base names, not paths.
>>> +ignore-patterns=common.py,
>>> +                doc.py,
>>> +                error.py,
>>> +                expr.py,
>>> +                gen.py,
>>> +                parser.py,
>>> +                schema.py,
>>> +                source.py,
>>> +                types.py,
>>> +                visit.py,
>> Already not ignored:
>>      __init__.py
>>      commands.py
>>      common.py
>>      debug.py
>>      events.py
>>      introspect.py
>>      script.py
>> Okay.
>> 
>>> +
>>> +
>>> +[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=fixme,
>>> +        missing-docstring,
>>> +        too-many-arguments,
>>> +        too-many-branches,
>>> +        too-many-statements,
>>> +        too-many-instance-attributes,
>> I'm fine with disabling these.
>> 
>
> I'd like to enable missing-docstring eventually, but that's not for today.

Understood.

>>> +
>>> +[REPORTS]
>>> +
>>> +[REFACTORING]
>>> +
>>> +[MISCELLANEOUS]
>>> +
>>> +[LOGGING]
>>> +
>>> +[BASIC]
>>> +
>>> +# Good variable names which should always be accepted, separated by a 
>>> comma.
>>> +good-names=i,
>>> +           j,
>>> +           k,
>>> +           ex,
>>> +           Run,
>>> +           _
>> Isn't this the default?
>> 
>
> Yes. I could omit it until I need to use good-names later on in the
> series, but I thought it would look odd to add the defaults at that
> point.
>
> So it's a minor bit of prescience here.

Matter of taste.  No objection.

>>> +
>>> +[VARIABLES]
>>> +
>>> +[STRING]
>>> +
>>> +[SPELLING]
>>> +
>>> +[FORMAT]
>>> +
>>> +[SIMILARITIES]
>>> +
>>> +# Ignore imports when computing similarities.
>>> +ignore-imports=yes
>> Why?
>> 
>
> We don't care if import statements are similar to those in other
> files. It's uninteresting entirely.
>
> (It matches on from typing import ... that exceed four lines, which I
> do regularly by the end of the series.)

What about something like

     # Ignore imports when computing similarities, because import
     # statements being similar is uninteresting entirely

>>> +
>>> +[TYPECHECK]
>>> +
>>> +[CLASSES]
>>> +
>>> +[IMPORTS]
>>> +
>>> +[DESIGN]
>>> +
>>> +[EXCEPTIONS]
>> Looks like you started with output of --generate-rcfile,
>> 
>
> I did,

Let's mention that in the commit message.  Education opportunity :)