This allows others to get repeatable results with pylint. If you run
`pylint iotests.py`, you should see a 100% pass.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/pylintrc | 22 ++++++++++++++++++++++
 1 file changed, 22 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..8720b6a0de
--- /dev/null
+++ b/tests/qemu-iotests/pylintrc
@@ -0,0 +1,22 @@
+[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,
+        no-else-return,
+        too-many-lines,
+        too-few-public-methods,
+        too-many-arguments,
+        too-many-locals,
+        too-many-branches,
+        too-many-public-methods,
+        # These are temporary, and should be removed:
+        missing-docstring,
+        line-too-long,
-- 
2.21.1

John Snow <jsnow@redhat.com> writes:

> This allows others to get repeatable results with pylint. If you run
> `pylint iotests.py`, you should see a 100% pass.

Nice.

>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/pylintrc | 22 ++++++++++++++++++++++
>  1 file changed, 22 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..8720b6a0de
> --- /dev/null
> +++ b/tests/qemu-iotests/pylintrc
> @@ -0,0 +1,22 @@
> +[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,
> +        no-else-return,
> +        too-many-lines,
> +        too-few-public-methods,
> +        too-many-arguments,
> +        too-many-locals,
> +        too-many-branches,
> +        too-many-public-methods,

Keep sorted?

> +        # These are temporary, and should be removed:
> +        missing-docstring,
> +        line-too-long,

For what it's worth, I also disable these for checking QAPI code, except
for no-else-return.  My true reason for keeping no-else-return is of
course that I agree with pylint these elses are ugly.  But I pretend to
simply go with the flow ;)

On 3/30/20 10:45 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> This allows others to get repeatable results with pylint. If you run
>> `pylint iotests.py`, you should see a 100% pass.
> 
> Nice.
> 
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/pylintrc | 22 ++++++++++++++++++++++
>>  1 file changed, 22 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..8720b6a0de
>> --- /dev/null
>> +++ b/tests/qemu-iotests/pylintrc
>> @@ -0,0 +1,22 @@
>> +[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,
>> +        no-else-return,
>> +        too-many-lines,
>> +        too-few-public-methods,
>> +        too-many-arguments,
>> +        too-many-locals,
>> +        too-many-branches,
>> +        too-many-public-methods,
> 
> Keep sorted?
> 

OK, (glances at email) if there's a good reason to respin. So far Kevin
asked me to change the word "atom" to "item" which I would consider a
"Maintainer, take mercy on me and just fix this up, would you?" fix, but
let's see how the rest of my email session goes today.

>> +        # These are temporary, and should be removed:
>> +        missing-docstring,
>> +        line-too-long,
> 
> For what it's worth, I also disable these for checking QAPI code, except
> for no-else-return.  My true reason for keeping no-else-return is of
> course that I agree with pylint these elses are ugly.  But I pretend to
> simply go with the flow ;)
> 

You probably also have a few more of the "too-many" warnings disabled,
too. I was working on refactoring the parser recently and found quite a
few more there that were just not worth fixing.

As for the no-else-return ... I think this is direly legitimate:

if cond_a:
    statement_1
    statement_2
    return statement_3
elif cond_b:
    statement_4
    statement_5
    return statement_6
else:
    statement_7
    statement_8
    return statement_9

When you've got at least three choices and neither one is particularly
"canonical", it can be nice to see them chunked out where they are
visually weighted equally.

That said, for this stuff:

if error_cond:
    return -1
else:
    return 0

Should probably be avoided, although ... it's fine.

I think no-else-return is just going to be an intense matter of style
and taste and cannot be left to the linter, so I think I agree with
leaving it on the disabled list.

--js

Am 25.03.2020 um 00:20 hat John Snow geschrieben:
> This allows others to get repeatable results with pylint. If you run
> `pylint iotests.py`, you should see a 100% pass.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

I see you said "should" and not "will", but I still get two warnings:

iotests.py:405:22: W0613: Unused argument 'signum' (unused-argument)
iotests.py:405:30: W0613: Unused argument 'frame' (unused-argument)

Kevin

On 3/30/20 11:49 AM, Kevin Wolf wrote:
> Am 25.03.2020 um 00:20 hat John Snow geschrieben:
>> This allows others to get repeatable results with pylint. If you run
>> `pylint iotests.py`, you should see a 100% pass.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
> I see you said "should" and not "will", but I still get two warnings:
> 
> iotests.py:405:22: W0613: Unused argument 'signum' (unused-argument)
> iotests.py:405:30: W0613: Unused argument 'frame' (unused-argument)
> 
> Kevin
> 

I wasn't entirely sure if it'd work across all versions of pylint, all
versions of python, or what the tree would look like by the time the
patches got applied ... and I didn't see these warnings.


I rebased to origin/master today and with these first five patches applied:

jsnow@probe ~/s/q/w/t/qemu-iotests (iotests-logging)> pylint iotests.py

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

jsnow@probe ~/s/q/w/t/qemu-iotests (iotests-logging)> pylint --version
pylint 2.3.1
astroid 2.2.5
Python 3.7.6 (default, Jan 30 2020, 10:29:04)
[GCC 9.2.1 20190827 (Red Hat 9.2.1-1)]


I upgraded to pylint 2.4.4 and that seems to work OK too.


So let's try this:

> sudo dnf install python36
> pipenv --python 3.6
> pipenv shell

> pipenv install pylint
> which pylint

(qemu-iotests) jsnow@probe ~/s/q/w/t/qemu-iotests (iotests-logging)>
which pylint
/home/jsnow/.local/share/virtualenvs/qemu-iotests-SREwaVl8/bin/pylint

(qemu-iotests) jsnow@probe ~/s/q/w/t/qemu-iotests (iotests-logging)>
pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.6.10 (default, Dec 27 2019, 13:40:13)
[GCC 9.2.1 20190827 (Red Hat 9.2.1-1)]


Python 3.6:
- Pylint 2.0.0 WARN
- Pylint 2.1.0 WARN
- Pylint 2.2.0 OK
- Pylint 2.4.4 OK
- Pylint 2.3.0 OK


Hm. Python 3.6 is from December 23, 2016 -- but Pylint from that era is
probably Pylint 1.6.5 from Jan 22, 2017.

That's probably ... obnoxiously old to support. Do we have any
viewpoints on minimum version requirements for CQA tools? Modern pylint
works just fine for Python 3.6, but I have no earthly idea if there are
reasons that "old" distributions with Python 3.6 can't get "modern"
pylint packages.

Gut feeling: Any Python 3.6 distribution has pip, and pip can get the
latest packages. It would be OK to require pylint >= 2.2.0, probably.


--js