diff mbox series

[v2,07/38] qapi: add pylintrc

Message ID 20200922210101.4081073-8-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: static typing conversion, pt1 | expand

Commit Message

John Snow Sept. 22, 2020, 9 p.m. UTC
Using `pylint --generate-rcfile > pylintrc`, generate a skeleton
pylintrc file. Sections that are not presently relevant (by the end of
this series) are removed leaving just the empty section as a search
engine / documentation hint to future authors.

Right now, quite a few modules are ignored as they are known to fail as
of this commit. modules 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 (for now). Due to a bug in pylint 2.5.x, pylint does not
correctly recognize when it is being run from "inside" a package, and
must be run *outside* of the package.

Therefore, to run it, you must:

 > 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

Comments

Eduardo Habkost Sept. 22, 2020, 9:54 p.m. UTC | #1
On Tue, Sep 22, 2020 at 05:00:30PM -0400, John Snow wrote:
> Using `pylint --generate-rcfile > pylintrc`, generate a skeleton
> pylintrc file. Sections that are not presently relevant (by the end of
> this series) are removed leaving just the empty section as a search
> engine / documentation hint to future authors.
> 
> Right now, quite a few modules are ignored as they are known to fail as
> of this commit. modules 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 (for now). Due to a bug in pylint 2.5.x, pylint does not
> correctly recognize when it is being run from "inside" a package, and
> must be run *outside* of the package.
> 
> Therefore, to run it, you must:
> 
>  > pylint scripts/qapi/ --rcfile=scripts/qapi/pylintrc
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Tested-by: Eduardo Habkost <ehabkost@redhat.com>

This doesn't generate any warnings after this patch, but at the
end of your -pt6 branch I got some pylint warnings.  I am
bisecting it to try to identify the patch where the warnings are
introduced.
Cleber Rosa Sept. 23, 2020, 1:42 p.m. UTC | #2
On Tue, Sep 22, 2020 at 05:00:30PM -0400, John Snow wrote:
> Using `pylint --generate-rcfile > pylintrc`, generate a skeleton
> pylintrc file. Sections that are not presently relevant (by the end of
> this series) are removed leaving just the empty section as a search
> engine / documentation hint to future authors.
> 
> Right now, quite a few modules are ignored as they are known to fail as
> of this commit. modules 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 (for now). Due to a bug in pylint 2.5.x, pylint does not
> correctly recognize when it is being run from "inside" a package, and
> must be run *outside* of the package.
> 
> Therefore, to run it, you must:
> 
>  > pylint scripts/qapi/ --rcfile=scripts/qapi/pylintrc
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

One concern I have here is that the pylint version is not defined.
Based on experience, different pylint will behave differently, because
among other things, it may introduce new checks.

I'd at the very least document the pylint version used in the commit
message, until a "requirements.txt"-like solution pinning a version is
given.

Other than that,

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
John Snow Sept. 23, 2020, 5:23 p.m. UTC | #3
On 9/23/20 9:42 AM, Cleber Rosa wrote:
> On Tue, Sep 22, 2020 at 05:00:30PM -0400, John Snow wrote:
>> Using `pylint --generate-rcfile > pylintrc`, generate a skeleton
>> pylintrc file. Sections that are not presently relevant (by the end of
>> this series) are removed leaving just the empty section as a search
>> engine / documentation hint to future authors.
>>
>> Right now, quite a few modules are ignored as they are known to fail as
>> of this commit. modules 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 (for now). Due to a bug in pylint 2.5.x, pylint does not
>> correctly recognize when it is being run from "inside" a package, and
>> must be run *outside* of the package.
>>
>> Therefore, to run it, you must:
>>
>>   > pylint scripts/qapi/ --rcfile=scripts/qapi/pylintrc
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
> 
> One concern I have here is that the pylint version is not defined.
> Based on experience, different pylint will behave differently, because
> among other things, it may introduce new checks.
> 
> I'd at the very least document the pylint version used in the commit
> message, until a "requirements.txt"-like solution pinning a version is
> given.
> 
> Other than that,
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> Tested-by: Cleber Rosa <crosa@redhat.com>
> 

Alright, I'll put it in the commit message itself instead of in the 
cover letter.

The next step is to re-engage on that Makefile patch that I was working 
on for ./python/qemu and introduce it here too, which will document the 
pinned versions correctly.

--js
Cleber Rosa Sept. 24, 2020, 7:29 p.m. UTC | #4
On Wed, Sep 23, 2020 at 01:23:56PM -0400, John Snow wrote:
> On 9/23/20 9:42 AM, Cleber Rosa wrote:
> > On Tue, Sep 22, 2020 at 05:00:30PM -0400, John Snow wrote:
> > > Using `pylint --generate-rcfile > pylintrc`, generate a skeleton
> > > pylintrc file. Sections that are not presently relevant (by the end of
> > > this series) are removed leaving just the empty section as a search
> > > engine / documentation hint to future authors.
> > > 
> > > Right now, quite a few modules are ignored as they are known to fail as
> > > of this commit. modules 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 (for now). Due to a bug in pylint 2.5.x, pylint does not
> > > correctly recognize when it is being run from "inside" a package, and
> > > must be run *outside* of the package.
> > > 
> > > Therefore, to run it, you must:
> > > 
> > >   > pylint scripts/qapi/ --rcfile=scripts/qapi/pylintrc
> > > 
> > > Signed-off-by: John Snow <jsnow@redhat.com>
> > 
> > One concern I have here is that the pylint version is not defined.
> > Based on experience, different pylint will behave differently, because
> > among other things, it may introduce new checks.
> > 
> > I'd at the very least document the pylint version used in the commit
> > message, until a "requirements.txt"-like solution pinning a version is
> > given.
> > 
> > Other than that,
> > 
> > Reviewed-by: Cleber Rosa <crosa@redhat.com>
> > Tested-by: Cleber Rosa <crosa@redhat.com>
> > 
> 
> Alright, I'll put it in the commit message itself instead of in the cover
> letter.
>

I missed that info on the cover letter, so my apologies.  But still, I
think it's a good idea to have that preserved in the repo history
indeed.

> The next step is to re-engage on that Makefile patch that I was working on
> for ./python/qemu and introduce it here too, which will document the pinned
> versions correctly.
> 
> --js

Agreed!

- Cleber.
diff mbox series

Patch

diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
new file mode 100644
index 0000000000..02dd22c863
--- /dev/null
+++ b/scripts/qapi/pylintrc
@@ -0,0 +1,74 @@ 
+[MASTER]
+
+# Add files or directories matching the regex patterns to the ignore list.
+# 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 import statements themselves when computing similarities.
+ignore-imports=yes
+
+[TYPECHECK]
+
+[CLASSES]
+
+[IMPORTS]
+
+[DESIGN]
+
+[EXCEPTIONS]