diff mbox series

[07/37] qapi: add pylintrc

Message ID 20200915224027.2529813-8-jsnow@redhat.com
State New
Headers show
Series qapi: static typing conversion, pt1 | expand

Commit Message

John Snow Sept. 15, 2020, 10:39 p.m. UTC
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
 > pylint qapi/ --rcfile=qapi/pylintrc

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/pylintrc | 74 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 scripts/qapi/pylintrc

Comments

Markus Armbruster Sept. 16, 2020, 12:30 p.m. UTC | #1
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

>

> 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.

> +

> +[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?

> +

> +[VARIABLES]

> +

> +[STRING]

> +

> +[SPELLING]

> +

> +[FORMAT]

> +

> +[SIMILARITIES]

> +

> +# Ignore imports when computing similarities.

> +ignore-imports=yes


Why?

> +

> +[TYPECHECK]

> +

> +[CLASSES]

> +

> +[IMPORTS]

> +

> +[DESIGN]

> +

> +[EXCEPTIONS]


Looks like you started with output of --generate-rcfile,
John Snow Sept. 16, 2020, 2:37 p.m. UTC | #2
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!

>>

>> 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.

>> +

>> +[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.

>> +

>> +[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.)

>> +

>> +[TYPECHECK]

>> +

>> +[CLASSES]

>> +

>> +[IMPORTS]

>> +

>> +[DESIGN]

>> +

>> +[EXCEPTIONS]

> 

> Looks like you started with output of --generate-rcfile,

> 


I did,
Markus Armbruster Sept. 17, 2020, 7:58 a.m. UTC | #3
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 :)
John Snow Sept. 17, 2020, 5:06 p.m. UTC | #4
On 9/17/20 3:58 AM, Markus Armbruster wrote:
>> 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

> 


OK. I left the "default" comments that pylint itself wrote. I'm now writing:

"Ignore import statements themselves when computing similarities."
diff mbox series

Patch

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,
+
+
+[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,
+
+[REPORTS]
+
+[REFACTORING]
+
+[MISCELLANEOUS]
+
+[LOGGING]
+
+[BASIC]
+
+# Good variable names which should always be accepted, separated by a comma.
+good-names=i,
+           j,
+           k,
+           ex,
+           Run,
+           _
+
+[VARIABLES]
+
+[STRING]
+
+[SPELLING]
+
+[FORMAT]
+
+[SIMILARITIES]
+
+# Ignore imports when computing similarities.
+ignore-imports=yes
+
+[TYPECHECK]
+
+[CLASSES]
+
+[IMPORTS]
+
+[DESIGN]
+
+[EXCEPTIONS]