test: Bump linter versions #19348

dunxen commented at 9:35 PM on June 21, 2020: contributor

As per #19346, mypy==0.700 was incompatible with Python 3.8.

I've bumped the versions of all the linters to their latest stable versions.

Checked with both Python 3.7 and 3.8 and everything still seems to work fine.

in test/README.md:263 in 5cc3d4cc66 outdated

 259 | @@ -260,11 +260,11 @@ Use the `-v` option for verbose output.
 260 |  
 261 |  | Lint test | Dependency | Version [used by CI](../ci/lint/04_install.sh) | Installation
 262 |  |-----------|:----------:|:-------------------------------------------:|--------------
 263 | -| [`lint-python.sh`](lint/lint-python.sh) | [flake8](https://gitlab.com/pycqa/flake8) | [3.7.8](https://github.com/bitcoin/bitcoin/pull/15257) | `pip3 install flake8==3.7.8`
 264 | -| [`lint-python.sh`](lint/lint-python.sh) | [mypy](https://github.com/python/mypy) | [0.700](https://github.com/bitcoin/bitcoin/pull/18210) | `pip3 install mypy==0.700`
 265 | -| [`lint-shell.sh`](lint/lint-shell.sh) | [ShellCheck](https://github.com/koalaman/shellcheck) | [0.6.0](https://github.com/bitcoin/bitcoin/pull/15166) | [details...](https://github.com/koalaman/shellcheck#installing)
 266 | +| [`lint-python.sh`](lint/lint-python.sh) | [flake8](https://gitlab.com/pycqa/flake8) | [3.8.3](https://github.com/bitcoin/bitcoin/pull/15257) | `pip3 install flake8==3.8.3`

dunxen commented at 9:36 PM on June 21, 2020:

Will modify these URLs now to point to this PR (#19348).

dunxen commented at 10:23 PM on June 21, 2020: contributor

Linting process working on Travis CI, but seems to show fail after test/lint/lint-python.sh with lint error E741 ambiguous variable name 'l'. Guess that's causing it to fail outright.

DrahtBot added the label Docs on Jun 21, 2020

DrahtBot added the label Tests on Jun 21, 2020

hebasto commented at 6:32 AM on June 22, 2020: member

Linting process working on Travis CI, but seems to show fail after test/lint/lint-python.sh with lint error E741 ambiguous variable name 'l'. Guess that's causing it to fail outright.

You could remove this line: https://github.com/bitcoin/bitcoin/blob/8ef15e8a86038225afef2487ca23abc10ca5dffa/test/lint/lint-python.sh#L42 in this PR. And in a followup restore it with fixing all warnings.

hebasto commented at 7:34 AM on June 22, 2020: member

From the ShellCheck v0.7.1 CHANGELOG:

SC2230: 'command -v' suggestion is now off by default (-i deprecate-which)

To be coherent we should

preserve the current behaviour and pass --enable=deprecate-which to https://github.com/bitcoin/bitcoin/blob/8ef15e8a86038225afef2487ca23abc10ca5dffa/test/lint/lint-shell.sh#L38

OR

accept new behaviour and remove this warnings from the disabled_gitian list https://github.com/bitcoin/bitcoin/blob/8ef15e8a86038225afef2487ca23abc10ca5dffa/test/lint/lint-shell.sh#L28

hebasto commented at 7:41 AM on June 22, 2020: member

Concept ACK.

practicalswift commented at 8:59 AM on June 22, 2020: contributor

Concept ACK

Nice first-time contribution @duncandean! Welcome as a contributor :)

dunxen commented at 9:03 AM on June 22, 2020: contributor

Thanks, @practicalswift :)

I'll make the above-mentioned changes when I get a gap today. @hebasto, in terms of ShellCheck, I'm leaning towards accepting the new behaviour and removing the warnings.

hebasto commented at 9:05 AM on June 22, 2020: member

@duncandean

@hebasto, in terms of ShellCheck, I'm leaning towards accepting the new behaviour and removing the warnings.

I'm OK with it :)

in test/lint/lint-python.sh:42 in 6ea83891b0 outdated

  38 | @@ -39,7 +39,6 @@ enabled=(
  39 |      E711 # comparison to None should be 'if cond is None:'
  40 |      E714 # test for object identity should be "is not"
  41 |      E721 # do not compare types, use "isinstance()"
  42 | -    E741 # do not use variables named "l", "O", or "I"

dunxen commented at 1:03 PM on June 22, 2020:

Going to add this back in a followup PR with fixes to offending code.

practicalswift commented at 5:49 PM on June 22, 2020:

Perhaps it can be done in this PR?

dunxen commented at 5:54 PM on June 22, 2020:

I guess that would make it more atomic :)

There doesn't seem to be too much going on in this PR so I am happy to fix these here too if wanted. 👍

hebasto commented at 6:01 PM on June 22, 2020:

Renaming variables to comply to E741, imo, deserves its own PR. It would be nice if it will a scripted-diff.

dunxen commented at 6:20 PM on June 22, 2020:

After checking, the usage is also not so uniform throughout. l is in place of listener, line, etc and there are occurrences of it as a function parameter. Seems there might be other further discussion on that too.

dunxen commented at 3:17 PM on June 22, 2020: contributor

rpc_createmultisig.py --descriptors failed. Not sure but looks like JSONRPC timeout.

laanwj commented at 5:33 PM on June 22, 2020: member

rpc_createmultisig.py --descriptors failed. Not sure but looks like JSONRPC timeout.

Yes, this failure is unrelated to linter version changes.

hebasto commented at 5:44 PM on June 22, 2020: member

6ea83891b025e69e382a89c5d144b5bd7f537ac6 LGTM

Mind squashing commits before the final review?

test: Bump linter versions

Updates Python linters, spellchecking, and ShellCheck versions. The PR links are updated for
the dependency versions in test/README.md. ShellCheck SC2230 removed to align with with new
behaviour in v0.7.1.

Fixes #19346.

39d526bde4

dunxen force-pushed on Jun 22, 2020

hebasto approved

hebasto commented at 6:29 PM on June 22, 2020: member

ACK 39d526bde48d98af4fa27906e85db0399b6aa8b1, I have reviewed the code and it looks OK, I agree it can be merged.

Also verified ShellCheck downloading URL and changelog.

MarcoFalke merged this on Jun 25, 2020

MarcoFalke closed this on Jun 25, 2020

dunxen deleted the branch on Nov 16, 2020

UdjinM6 referenced this in commit 731152633e on Sep 10, 2021

UdjinM6 referenced this in commit 1bbb2f8ec2 on Sep 24, 2021

UdjinM6 referenced this in commit 88c5511bf6 on Sep 24, 2021

UdjinM6 referenced this in commit f72167206d on Oct 4, 2021

UdjinM6 referenced this in commit 078ae49dc7 on Oct 5, 2021

kittywhiskers referenced this in commit 77c49f8b18 on Oct 12, 2021

pravblockc referenced this in commit 7602685ffb on Nov 18, 2021

DrahtBot locked this on Feb 15, 2022