Enable flake8 rule E225 which checks for missing whitespace around op… #14534

1. 
   jbampton commented at 10:16 pm on October 20, 2018: contributor

   …erators

   Lint Python code for rule E225

2. jbampton force-pushed on Oct 20, 2018
3. in .gitignore:120 in 74f4e2d648 outdated

   115@@ -116,3 +116,5 @@ test/cache/*
   116 
   117 libbitcoinconsensus.pc
   118 contrib/devtools/split-debug.sh
   119+
   120+.idea
   

   ---

   ---

   

   practicalswift commented at 11:57 pm on October 20, 2018:

   Remove this. See “Ignoring IDE/editor files” in the developer notes :-)
4. fanquake added the label Tests on Oct 20, 2018
5. 
   practicalswift commented at 0:00 am on October 21, 2018: contributor

   ACK 74f4e2d648eb40dd2315b46d87d6824f9f419e56 assuming nit is addressed and squash :-)
6. Enable flake8 rule E225 which checks for missing whitespace around operators

   Lint Python code for rule E225

   0ad199b543
7. jbampton force-pushed on Oct 21, 2018
8. 
   jbampton commented at 1:25 am on October 21, 2018: contributor

   Hey @practicalswift the nit is fixed and now squashed 👍
9. 
   DrahtBot commented at 3:45 am on October 21, 2018: member

   • #14296 ([wallet] Remove addwitnessaddress by jnewbery)
   • #14066 (gitian-linux: Build binaries for 64-bit POWER by luke-jr)
   • #13998 (Scripts and tools: gitian-build.py improvements and corrections by hebasto)

   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.

10. 
    practicalswift commented at 10:09 am on October 21, 2018: contributor

    ACK 0ad199b543465f6aead05ee9014907eca36049e5

    Nice to never have to think about or comment on this ever again in future code reviews thanks to this check: review automation FTW :-)

11. 
    Empact commented at 11:08 pm on October 21, 2018: member

    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
12. 
    ken2812221 commented at 11:49 pm on October 21, 2018: contributor

    It would be better to use scripted-diff for this kind of change. Use autopep8 or something else can auto format.
13. 
    jbampton commented at 8:08 am on October 22, 2018: contributor

    Hey everyone, I did use autopep8 for this.

    From the project root directory the command I ran was:

    0autopep8 --in-place --select=E225 -r .
    

    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:

    https://github.com/hhatto/autopep8

14. 
    ken2812221 commented at 11:58 am on October 22, 2018: contributor

    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):

    0-BEGIN VERIFY SCRIPT-
    1pip3 install --user autopep8
    2autopep8 --in-place --select=E225 -r .
    3-END VERIFY SCRIPT-
    

15. 
    jbampton commented at 1:20 pm on October 22, 2018: contributor

    Hey @ken2812221 thanks for explaining the process further.

    Does that mean you want me to commit again with a scripted diff ?

16. 
    promag commented at 6:16 pm on October 22, 2018: member

    How can we ensure other pulls don’t break master if merged after this?

    +1 scripted diff.

    Edit: @ken2812221 install in the script?

17. 
    ken2812221 commented at 1:09 am on October 23, 2018: contributor

    @jbampton You can git commit --amend to rewrite the commit message and force push it. @promag Yes, I assume travis does not have autopep8 installed. Also we don’t want to change travis script. I think this is the only way to install it.
18. 
    MarcoFalke commented at 8:30 pm on October 23, 2018: member

    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.
19. 
    MarcoFalke commented at 8:30 pm on October 23, 2018: member

    See also #14540
20. 
    DrahtBot commented at 1:26 pm on October 24, 2018: member
21. DrahtBot added the label Needs rebase on Oct 24, 2018
22. 
    MarcoFalke commented at 3:41 pm on October 26, 2018: member

    Trivial pull request still needs rebase. Closing for now.
23. MarcoFalke closed this on Oct 26, 2018

    ---

24. jbampton deleted the branch on Oct 14, 2019
25. laanwj removed the label Needs rebase on Oct 24, 2019
26. DrahtBot locked this on Dec 16, 2021

Contributors
jbampton practicalswift DrahtBot Empact ken2812221 promag MarcoFalke

Labels
Tests