Enable flake8 rule E225 which checks for missing whitespace around op…#14534
Enable flake8 rule E225 which checks for missing whitespace around op…#14534jbampton wants to merge 1 commit intobitcoin:masterfrom
Conversation
f9e1108 to
8832684
Compare
|
ACK 74f4e2d648eb40dd2315b46d87d6824f9f419e56 assuming nit is addressed and squash :-) |
…erators Lint Python code for rule E225
fbe2eb0 to
0ad199b
Compare
|
Hey @practicalswift the nit is fixed and now squashed 👍 |
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
ACK 0ad199b Nice to never have to think about or comment on this ever again in future code reviews thanks to this check: review automation FTW :-) |
|
utACK 0ad199b changes are whitespace only (https://github.com/bitcoin/bitcoin/pull/14534/files?w=1), the result is easier to read and the practice is automatically enforced |
|
It would be better to use scripted-diff for this kind of change. Use autopep8 or something else can auto format. |
|
Hey everyone, I did use autopep8 for this. From the project root directory the command I ran was: I would be happy to add other flake8 checks if you want to this pull request. autopep8 uses pycodestyle https://pep8.readthedocs.io/en/latest/intro.html#error-codes Here is the link to autopep8: |
|
What I mean is writing the commands you were using into the commit message by scripted-diff. Then travis-ci will run the commands again to check if it is the same. So reviewers won't have to run the commands manually on their own computer to check if the change is correct or not. For example (Haven't tested): |
|
Hey @ken2812221 thanks for explaining the process further. Does that mean you want me to commit again with a scripted diff ? |
|
How can we ensure other pulls don't break master if merged after this? +1 scripted diff. Edit: @ken2812221 install in the script? |
|
Both variants are allowed in python and the whitespace is not really "missing". Tend to NACK, as I don't see the advantage of doing this code churn for everyone. |
|
See also #14540 |
| Needs rebase |
|
Trivial pull request still needs rebase. Closing for now. |
…erators
Lint Python code for rule E225