tests/tools: Enable additional Python flake8 rules for automatic linting via Travis by practicalswift · Pull Request #12987 · bitcoin/bitcoin

practicalswift · 2018-04-15T11:02:33Z

Enabled rules:

* E242: tab after ','
* E266: too many leading '#' for block comment
* E401: multiple imports on one line
* E402: module level import not at top of file
* E701: multiple statements on one line (colon)
* E901: SyntaxError: invalid syntax
* E902: TokenError: EOF in multi-line string
* F821: undefined name 'Foo'
* W293: blank line contains whitespace
* W606: 'async' and 'await' are reserved keywords starting with Python 3.7

Note to reviewers:

In general we don't allow whitespace cleanups to existing code, but in order to allow for enabling Travis checking for these rules a few smaller whitespace cleanups had to made as part of this PR.
Use this ?w=1 link to show a diff without whitespace changes.

Before this commit:

$ flake8 -qq --statistics --ignore=B,C,E,F,I,N,W --select=E112,E113,E115,E116,E125,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,F401,E901,E902,F402,F404,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W292,W293,W504,W601,W602,W603,W604,W605,W606 .
5     E266 too many leading '#' for block comment
4     E401 multiple imports on one line
6     E402 module level import not at top of file
5     E701 multiple statements on one line (colon)
1     F812 list comprehension redefines 'n' from line 159
4     F821 undefined name 'ConnectionRefusedError'
28    W293 blank line contains whitespace

After this commit:

$ flake8 -qq --statistics --ignore=B,C,E,F,I,N,W --select=E112,E113,E115,E116,E125,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,F401,E901,E902,F402,F404,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W292,W293,W504,W601,W602,W603,W604,W605,W606 .
$

maflcko

I am fine with removing the trailing white space, since that is now enforced for a couple of months. Though, I don't like to enforce strict rules on leading white space or statements that are otherwise identical.

maflcko · 2018-04-15T11:54:12Z

nack. There is no way we can force everyone to configure their editor in this specific way.

I've now reverted this change and the corresponding rule (E101: indentation contains mixed spaces and tabs).

maflcko · 2018-04-15T11:54:42Z

Those seem identical. No reason to enforce either and forbid the other.

maflcko · 2018-04-15T11:56:00Z

Same here and the hunk above. (No way we can force everyone to configure their editors to behave this way)

practicalswift · 2018-04-15T16:25:58Z

@MarcoFalke Thanks for reviewing. Good feedback! Now updated. Please re-review :-)

maflcko · 2018-04-15T16:42:55Z

Other than W293, I'd drop all those that seem to enforce white space in a specific way. I'd rather avoid being extra strict on everyone and prescribe exactly how to use whitespace "properly".

practicalswift · 2018-04-15T16:54:37Z

@MarcoFalke Thanks for clarifying!

Of the whitespace related rule changes these can be divided into 1.) indentation related and 2.) related to whitespace around operators.

Indentation related:

1     E121 continuation line under-indented for hanging indent
1     E123 closing bracket does not match indentation of opening bracket's line
1     E124 closing bracket does not match visual indentation
2     E126 continuation line over-indented for hanging indent
1     E129 visually indented line with same indent as next logical line

Related to whitespace around operators:

7     E222 multiple spaces after operator
4     E228 missing whitespace around modulo operator

Do you want to get rid of all rules above, or only the indentation related? :-)

maflcko · 2018-04-15T17:19:44Z

Personally, I'd prefer to get rid of all of them for now. (If you or someone else feels strongly about them, they can put them in a separate pull request)

practicalswift · 2018-04-16T07:01:56Z

@MarcoFalke Updated! Please re-review :-)

fanquake · 2018-04-16T08:47:08Z

from exceptions import BrokenPipeError, ConnectionResetError ImportError: No module named 'exceptions'

maflcko · 2018-04-16T12:01:01Z

Might as well remove the whole lines, since they are redundant.

maflcko · 2018-04-16T12:04:46Z

utACK 21cc7ddc0406ee3c76623e7f23dbbcd2d0b6f241

practicalswift · 2018-04-16T12:32:53Z

@MarcoFalke Updated! Please re-re-review :-)

maflcko · 2018-04-16T13:15:31Z

re-utACK bf0d779

jnewbery · 2018-04-16T14:51:01Z

utACK 96455458ab3f7183141eb64d6e7c3abe11c7a355. Seems like useful cleanup.

maflcko · 2018-04-16T15:19:05Z

Please fixup the travis failure.

practicalswift · 2018-04-16T15:41:04Z

@MarcoFalke Thanks for the ping! Fixed!

Please re-review :-)

jnewbery · 2018-04-16T15:45:03Z

utACK 643aad1

…automatic linting via Travis 643aad1 Enable additional flake8 rules (practicalswift) f020aca Minor Python cleanups to make flake8 pass with the new rules enabled (practicalswift) Pull request description: Enabled rules: ``` * E242: tab after ',' * E266: too many leading '#' for block comment * E401: multiple imports on one line * E402: module level import not at top of file * E701: multiple statements on one line (colon) * E901: SyntaxError: invalid syntax * E902: TokenError: EOF in multi-line string * F821: undefined name 'Foo' * W293: blank line contains whitespace * W606: 'async' and 'await' are reserved keywords starting with Python 3.7 ``` Note to reviewers: * In general we don't allow whitespace cleanups to existing code, but in order to allow for enabling Travis checking for these rules a few smaller whitespace cleanups had to made as part of this PR. * Use [this `?w=1` link](https://github.com/bitcoin/bitcoin/pull/12987/files?w=1) to show a diff without whitespace changes. Before this commit: ``` $ flake8 -qq --statistics --ignore=B,C,E,F,I,N,W --select=E112,E113,E115,E116,E125,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,F401,E901,E902,F402,F404,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W292,W293,W504,W601,W602,W603,W604,W605,W606 . 5 E266 too many leading '#' for block comment 4 E401 multiple imports on one line 6 E402 module level import not at top of file 5 E701 multiple statements on one line (colon) 1 F812 list comprehension redefines 'n' from line 159 4 F821 undefined name 'ConnectionRefusedError' 28 W293 blank line contains whitespace ``` After this commit: ``` $ flake8 -qq --statistics --ignore=B,C,E,F,I,N,W --select=E112,E113,E115,E116,E125,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,F401,E901,E902,F402,F404,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W292,W293,W504,W601,W602,W603,W604,W605,W606 . $ ``` Tree-SHA512: fc7d5e752298a50d4248afc620ee2c173135b4ca008e48e02913ac968e5a24a5fd5396926047ec62f1d580d537434ccae01f249bb2f3338fa59dc630bf97ca7a

…es for automatic linting via Travis 643aad1 Enable additional flake8 rules (practicalswift) f020aca Minor Python cleanups to make flake8 pass with the new rules enabled (practicalswift) Pull request description: Enabled rules: ``` * E242: tab after ',' * E266: too many leading '#' for block comment * E401: multiple imports on one line * E402: module level import not at top of file * E701: multiple statements on one line (colon) * E901: SyntaxError: invalid syntax * E902: TokenError: EOF in multi-line string * F821: undefined name 'Foo' * W293: blank line contains whitespace * W606: 'async' and 'await' are reserved keywords starting with Python 3.7 ``` Note to reviewers: * In general we don't allow whitespace cleanups to existing code, but in order to allow for enabling Travis checking for these rules a few smaller whitespace cleanups had to made as part of this PR. * Use [this `?w=1` link](https://github.com/bitcoin/bitcoin/pull/12987/files?w=1) to show a diff without whitespace changes. Before this commit: ``` $ flake8 -qq --statistics --ignore=B,C,E,F,I,N,W --select=E112,E113,E115,E116,E125,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,F401,E901,E902,F402,F404,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W292,W293,W504,W601,W602,W603,W604,W605,W606 . 5 E266 too many leading '#' for block comment 4 E401 multiple imports on one line 6 E402 module level import not at top of file 5 E701 multiple statements on one line (colon) 1 F812 list comprehension redefines 'n' from line 159 4 F821 undefined name 'ConnectionRefusedError' 28 W293 blank line contains whitespace ``` After this commit: ``` $ flake8 -qq --statistics --ignore=B,C,E,F,I,N,W --select=E112,E113,E115,E116,E125,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,F401,E901,E902,F402,F404,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W292,W293,W504,W601,W602,W603,W604,W605,W606 . $ ``` Tree-SHA512: fc7d5e752298a50d4248afc620ee2c173135b4ca008e48e02913ac968e5a24a5fd5396926047ec62f1d580d537434ccae01f249bb2f3338fa59dc630bf97ca7a Signed-off-by: pasta <pasta@dashboost.org>

fanquake added the Scripts and tools label Apr 15, 2018

practicalswift changed the title ~~tests/python: Enable additional Python flake8 rules for automatic linting via Travis~~ tests/scripts: Enable additional Python flake8 rules for automatic linting via Travis Apr 15, 2018

practicalswift changed the title ~~tests/scripts: Enable additional Python flake8 rules for automatic linting via Travis~~ tests/tools: Enable additional Python flake8 rules for automatic linting via Travis Apr 15, 2018

practicalswift force-pushed the enable-flake8-checks branch 3 times, most recently from 41caac9 to 2757516 Compare April 15, 2018 11:57

maflcko suggested changes Apr 15, 2018

View reviewed changes

practicalswift force-pushed the enable-flake8-checks branch 2 times, most recently from 53fc83b to 10fd16d Compare April 15, 2018 16:12

practicalswift force-pushed the enable-flake8-checks branch from 10fd16d to 1e87b26 Compare April 16, 2018 06:58

practicalswift force-pushed the enable-flake8-checks branch 3 times, most recently from 4bdfc30 to 3b7e497 Compare April 16, 2018 07:58

fanquake reviewed Apr 16, 2018

View reviewed changes

practicalswift force-pushed the enable-flake8-checks branch from 3b7e497 to 21cc7dd Compare April 16, 2018 09:14

maflcko reviewed Apr 16, 2018

View reviewed changes

practicalswift force-pushed the enable-flake8-checks branch 2 times, most recently from a30b48d to bf0d779 Compare April 16, 2018 12:16

practicalswift force-pushed the enable-flake8-checks branch from bf0d779 to 9645545 Compare April 16, 2018 14:40

Minor Python cleanups to make flake8 pass with the new rules enabled

f020aca

Enable additional flake8 rules

643aad1

practicalswift force-pushed the enable-flake8-checks branch from 9645545 to 643aad1 Compare April 16, 2018 15:25

maflcko merged commit 643aad1 into bitcoin:master Apr 16, 2018

Bushstar mentioned this pull request Apr 17, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#298

Merged

asottile mentioned this pull request Apr 3, 2021

Please add release notes PyCQA/flake8#273

Closed

practicalswift deleted the enable-flake8-checks branch April 10, 2021 19:34

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Uh oh!

Conversation

practicalswift commented Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko Apr 15, 2018

Choose a reason for hiding this comment

Uh oh!

practicalswift Apr 15, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko Apr 15, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko Apr 15, 2018

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Apr 15, 2018

Uh oh!

maflcko commented Apr 15, 2018

Uh oh!

practicalswift commented Apr 15, 2018

Uh oh!

maflcko commented Apr 15, 2018

Uh oh!

practicalswift commented Apr 16, 2018

Uh oh!

fanquake Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko commented Apr 16, 2018

Uh oh!

practicalswift commented Apr 16, 2018

Uh oh!

maflcko commented Apr 16, 2018

Uh oh!

jnewbery commented Apr 16, 2018

Uh oh!

maflcko commented Apr 16, 2018

Uh oh!

practicalswift commented Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnewbery commented Apr 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

practicalswift commented Apr 15, 2018 •

edited

Loading

practicalswift commented Apr 16, 2018 •

edited

Loading