Scripts and tools: Bump flake8 to 3.7.8 #15257

pull Empact wants to merge 4 commits into bitcoin:master from Empact:flake-36 changing 16 files +39 −43
  1. Empact commented at 7:31 am on January 25, 2019: member

    This is a second go at #15221, fixing new lints in: W504 line break after binary operator W605 invalid escape sequence F841 local variable ’e’ is assigned to but never used

    This time around:

    • One commit per rule, for easier review
    • I went with the PEP-8 style of breaking before binary operators
    • I looked into the raw regex newline issue, and found that raw strings with newlines embedded do work appropriately. E.g. run re.match(r" \n ", " \n ") to check this for yourself. re.MULTILINE exists to modify ^ and $ in multiline scenarios, but all of these searches are per-line.
  2. fanquake added the label Tests on Jan 25, 2019
  3. fanquake added the label Scripts and tools on Jan 25, 2019
  4. practicalswift commented at 8:38 am on January 25, 2019: contributor

    Concept ACK

    Thanks for doing this!

  5. promag commented at 9:56 am on January 25, 2019: member
    Concept ACK.
  6. in contrib/devtools/copyright_header.py:84 in cf494074d8 outdated
    105-    "Sam Rushing\n",
    106-    "ArtForz -- public domain half-a-node\n",
    107-    "Intel Corporation",
    108-    "The Zcash developers",
    109-    "Jeremy Rubin",
    110+    r"Satoshi Nakamoto\n",
    


    laanwj commented at 4:13 pm on January 25, 2019:

    As I commented on your previous request, turning these into raw strings is not correct. This is not a pure refactor and changes the meaning of the code.

    0>>> r"Satoshi Nakamoto\n" == "Satoshi Nakamoto\n"
    1False
    

    Raw strings are unnecessary here. If you want to avoid the warning stop escaping . as \,, e.g.

    0- "The LevelDB Authors\. All rights reserved\.\n",
    1+ "The LevelDB Authors\. All rights reserved.\n",
    

    Empact commented at 9:45 am on January 26, 2019:

    If you run those strings against re instead you’ll see something different. E.g. via re.match:

     0>>> import re
     1>>> re.match(".", "K") # standard string, no escape, matches any character
     2<re.Match object; span=(0, 1), match='K'>
     3>>> re.match("\.", "K") # standard string, with escape, does not match any character (hence no result)
     4>>> re.match("\.", ".") # standard string, with escape, does match "."
     5<re.Match object; span=(0, 1), match='.'>
     6>>> re.match(r"\.", "K") # raw string, with escape, does not match any character
     7>>> re.match(r"\.", ".") # raw string, with escape, does match "."
     8<re.Match object; span=(0, 1), match='.'>
     9>>> re.match("\\.", "K") # standard string, double escape, does not match any character
    10>>> re.match("\\.", ".") # standard string, double escape, does match "."
    11<re.Match object; span=(0, 1), match='.'>
    12>>> re.match(r"\\.", "K") # raw string, double escape, does not match any character
    13>>> re.match(r"\\.", ".") # raw string, double escape, does not match "."
    

    You can see above that absent the \, “.” is interpreted as any character, while “\.”, r"\." and “\\.” are both interpreted as an escaped period. Python is planning to error on “\.” in the future. https://bugs.python.org/issue27364


    Empact commented at 9:59 am on January 26, 2019:

    Another way to look at this is comparing the script output with master. The output of contrib/devtools/copyright_header.py report . from project root currently matches between current master (https://github.com/bitcoin/bitcoin/commit/ab46fe6ec1b37e88c5a06ee7a06ab31cbd18304f) and this branch (https://github.com/bitcoin/bitcoin/pull/15257/commits/8f0b710f4f0e5087eba73cd047883353a25188ac) - but for one character, escaping the . in 'Wladimir J\. van der Laan\n', in keeping with the above, the matches are the same:

     0-------------------------------------------------------------------------------
     1667 files examined according to INCLUDE and EXCLUDE fnmatch rules
     2-------------------------------------------------------------------------------
     3
     4  18 with zero copyrights
     5 504 with one copyright
     6 141 with two copyrights
     7   4 with three copyrights
     8   0 with four or more copyrights
     9
    10-------------------------------------------------------------------------------
    11Copyrights with dominant style:
    12e.g. "Copyright (c)" and "<year>" or "<startYear>-<endYear>":
    13
    14 135 with 'Satoshi Nakamoto\n'
    15 635 with 'The Bitcoin Core developers\n'
    16   1 with 'Bitcoin Core Developers\n'
    17   1 with 'University of Illinois at Urbana-Champaign\.\n'
    18   6 with 'Pieter Wuille\n'
    19   3 with 'Wladimir J\. van der Laan\n'
    20   3 with 'Jeff Garzik\n'
    21   1 with 'Jan-Klaas Kollhof\n'
    22   1 with 'Sam Rushing\n'
    23   2 with 'ArtForz -- public domain half-a-node\n'
    24   1 with 'Intel Corporation'
    25   6 with 'The Zcash developers'
    26   1 with 'Jeremy Rubin'
    27
    28-------------------------------------------------------------------------------
    29Copyrights with year list style:
    30e.g. "Copyright (c)" and "<year1>, <year2>, ...":
    31
    32
    33-------------------------------------------------------------------------------
    34Copyrights with no "(c)" style:
    35e.g. "Copyright" and "<year>" or "<startYear>-<endYear>":
    36
    37   1 with 'The Bitcoin Core developers\n'
    38   1 with 'BitPay Inc\.\n'
    39
    40-------------------------------------------------------------------------------
    410 with unexpected copyright holder names
    42-------------------------------------------------------------------------------
    

    laanwj commented at 12:18 pm on January 31, 2019:
    Okay then I was apparently misunderstanding what this is trying to do in the first place, sorry.

    Empact commented at 4:02 pm on January 31, 2019:
    All good, thanks!
  7. DrahtBot commented at 3:05 am on January 26, 2019: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)

    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.

  8. Empact force-pushed on Jan 26, 2019
  9. Empact force-pushed on Jan 26, 2019
  10. DrahtBot added the label Needs rebase on Jan 26, 2019
  11. Empact force-pushed on Jan 27, 2019
  12. Empact commented at 3:54 am on January 27, 2019: member
    Rebased for #15258
  13. fanquake removed the label Needs rebase on Jan 27, 2019
  14. Empact force-pushed on Jan 31, 2019
  15. Empact force-pushed on Jan 31, 2019
  16. Empact commented at 11:00 pm on January 31, 2019: member
    @MarcoFalke I pulled in your commit and it covered these additional cases: 59ffecf66cf4d08c4b431e457b083878d66a3fd6. The trouble is, it’s not comprehensive and it’s not easy to create a comprehensive scripted-diff given regex strings are not always directly connected with re. calls. I think limiting to addressing the flake warnings is reasonable given that.
  17. Empact force-pushed on Jan 31, 2019
  18. Empact commented at 11:04 pm on January 31, 2019: member
    Rebased
  19. in contrib/devtools/copyright_header.py:98 in d79bee7a5e outdated
     96+    r"BitPay Inc\.\n",
     97+    r"University of Illinois at Urbana-Champaign\.\n",
     98     "Pieter Wuille\n",
     99-    "Wladimir J. van der Laan\n",
    100+    r"Wladimir J\. van der Laan\n",
    101     "Jeff Garzik\n",
    


    MarcoFalke commented at 2:16 pm on February 1, 2019:
    It seems inconsistent to have some lines without a newline, some with a \n and others with a \\n. I’d prefer to use the same suffix for all (or leave it and append the suffix at runtime)
  20. practicalswift commented at 2:28 pm on February 1, 2019: contributor
    utACK d79bee7a5e2aa198f44472ba834be1be16f95c2d modulo @MarcoFalke nit
  21. Empact force-pushed on Feb 1, 2019
  22. Empact force-pushed on Feb 1, 2019
  23. Empact force-pushed on Feb 2, 2019
  24. in contrib/devtools/copyright_header.py:88 in f45fde893c outdated
    84@@ -85,23 +85,23 @@ def get_filenames_to_examine(base_directory):
    85 ANY_COPYRIGHT_COMPILED = re.compile(ANY_COPYRIGHT_STYLE_OR_YEAR_STYLE)
    86 
    87 def compile_copyright_regex(copyright_style, year_style, name):
    88-    return re.compile('%s %s,? %s' % (copyright_style, year_style, name))
    89+    return re.compile('%s %s,? %s\n' % (copyright_style, year_style, name))
    


    JustinTArthur commented at 10:43 pm on February 13, 2019:
    Should this be a raw string literal now as well?

    Empact commented at 10:37 am on February 26, 2019:
    It’s not strictly required since the deprecation is about invalid escape sequences, and \n is valid. But the change in https://github.com/bitcoin/bitcoin/pull/15257/commits/45760242428fa53ba2719bbc35a68608cc4fc972 makes it necessary (because \* is an invalid escape sequence).
  25. laanwj added this to the milestone 0.19.0 on Feb 21, 2019
  26. laanwj commented at 8:53 am on February 21, 2019: member
    Added 0.19 milestone, I have the feeling this going to need some iterations and thus too risky for 0.18.
  27. Empact force-pushed on Feb 26, 2019
  28. Empact renamed this:
    Scripts and tools: Bump flake8 to 3.6.0
    Scripts and tools: Bump flake8 to 3.7.7
    on Feb 26, 2019
  29. Empact commented at 10:36 am on February 26, 2019: member
    Rebased, bumped to 3.7.7, fixed additional lints. Added another commit to fix a copyright detection in libsecp256k1_config.h.
  30. DrahtBot added the label Needs rebase on Apr 29, 2019
  31. Empact force-pushed on May 3, 2019
  32. DrahtBot removed the label Needs rebase on May 3, 2019
  33. Empact force-pushed on Jun 9, 2019
  34. DrahtBot added the label Needs rebase on Jun 18, 2019
  35. Empact force-pushed on Jun 22, 2019
  36. Empact commented at 5:49 am on June 22, 2019: member
    Rebased
  37. DrahtBot removed the label Needs rebase on Jun 22, 2019
  38. practicalswift commented at 7:38 am on June 22, 2019: contributor
    utACK efc39ea93990d1477988b0a9079afc788f64e3dc
  39. DrahtBot added the label Needs rebase on Jul 3, 2019
  40. Empact force-pushed on Jul 8, 2019
  41. DrahtBot removed the label Needs rebase on Jul 9, 2019
  42. Empact renamed this:
    Scripts and tools: Bump flake8 to 3.7.7
    Scripts and tools: Bump flake8 to 3.7.8
    on Jul 10, 2019
  43. Empact commented at 5:29 am on July 10, 2019: member
    Rebased, bumped to 3.7.8.
  44. practicalswift commented at 9:16 am on July 10, 2019: contributor
    utACK 58054fb1c703f7602ef24470e37e44b4a9db4dbf
  45. in contrib/devtools/copyright_header.py:45 in 58054fb1c7 outdated
    40@@ -41,8 +41,8 @@ def applies_to_file(filename):
    41     for excluded_dir in EXCLUDE_DIRS:
    42         if filename.startswith(excluded_dir):
    43             return False
    44-    return ((EXCLUDE_COMPILED.match(filename) is None) and
    45-            (INCLUDE_COMPILED.match(filename) is not None))
    46+    return ((EXCLUDE_COMPILED.match(filename) is None)
    47+            and (INCLUDE_COMPILED.match(filename) is not None))
    


    MarcoFalke commented at 11:32 am on July 10, 2019:
    W504 should be disabled. This is not a critical error that should be blocking a merge
  46. DrahtBot added the label Needs rebase on Aug 15, 2019
  47. test/contrib: Fix invalid escapes in regex strings
    Flagged by flake8 v3.6.0, as W605, plus a few others identified
    incidentally, e.g. 59ffecf66cf4d08c4b431e457b083878d66a3fd6.
    
    Note that r"\n" matches to "\n" under re.match/search.
    b21680baf5
  48. lint: Disable flake8 W504 warning
    In the words of MarcoFalke:
    "W504 should be disabled. This is not a critical error that should be blocking a merge"
    https://github.com/bitcoin/bitcoin/pull/15257#discussion_r302017280
    838920704a
  49. lint: Bump flake8 to 3.7.8 0ef0e51fe4
  50. devtools: Accomodate block-style copyright blocks
    Without this, `copyright_header.py report . verbose` reports:
    -------------------------------------------------------------------------------
    1 with unexpected copyright holder names
    	./build_msvc/libsecp256k1_config.h
    -------------------------------------------------------------------------------
    3d0a82cff8
  51. Empact force-pushed on Sep 3, 2019
  52. Empact commented at 6:45 pm on September 3, 2019: member
    Rebased and disabled W504
  53. DrahtBot removed the label Needs rebase on Sep 3, 2019
  54. practicalswift commented at 7:06 pm on September 3, 2019: contributor
    ACK 3d0a82cff8cbb809876e82dbe62d14d2adc07d94 – diff looks correct
  55. MarcoFalke referenced this in commit 45be44cce4 on Sep 5, 2019
  56. MarcoFalke merged this on Sep 5, 2019
  57. MarcoFalke closed this on Sep 5, 2019

  58. HashUnlimited referenced this in commit 045aa4ecd1 on Sep 5, 2019
  59. t4n6a1ka referenced this in commit 602f194fe2 on Sep 12, 2019
  60. Empact deleted the branch on Sep 26, 2019
  61. PierreRochard referenced this in commit 4279af3cf7 on Oct 12, 2019
  62. barrystyle referenced this in commit 056484c026 on Nov 11, 2019
  63. UdjinM6 referenced this in commit dc815a5c84 on Sep 10, 2021
  64. UdjinM6 referenced this in commit cca3dd9468 on Sep 24, 2021
  65. UdjinM6 referenced this in commit e6a3fbd23a on Oct 4, 2021
  66. UdjinM6 referenced this in commit 60b3c5a64e on Oct 5, 2021
  67. kittywhiskers referenced this in commit e41e5996d6 on Oct 12, 2021
  68. pravblockc referenced this in commit 71f0b12bf2 on Nov 18, 2021
  69. DrahtBot locked this on Dec 16, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-29 04:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me