Skip to content

Commit

Permalink
[lint] remove the --preview flag when running black
Browse files Browse the repository at this point in the history 
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
  • Loading branch information
@PiRK
PiRK committed Jul 21, 2023
1 parent af2856c commit a5d12e2
Show file tree
Hide file tree
Showing 67 changed files with 396 additions and 543 deletions.
3 changes: 0 additions & 3 deletions .arclint
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@
"(^contrib/apple-sdk-tools/)",
"(^electrum/electrumabc_gui/qt/icons.py)",
"(\\_pb2.py$)"
],
"flags": [
"--preview"
]
},
"flake8": {
Expand Down
27 changes: 15 additions & 12 deletions contrib/buildbot/phabricator_wrapper.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,17 +190,17 @@ def decorateCommitMap(self, commitMapIn):
for rev in revs:
if revisionPHID == rev["phid"]:
decoratedCommit["revision"] = rev
decoratedCommit["link"] = (
f"https://reviews.bitcoinabc.org/D{rev['id']}"
)
decoratedCommit[
"link"
] = f"https://reviews.bitcoinabc.org/D{rev['id']}"
break

for author in authors:
if author["phid"] == rev["fields"]["authorPHID"]:
decoratedCommit["author"] = author
decoratedCommit["authorSlackUsername"] = (
self.getAuthorSlackUsername(author)
)
decoratedCommit[
"authorSlackUsername"
] = self.getAuthorSlackUsername(author)
break
commitMap[commitHash] = decoratedCommit
return commitMap
Expand Down Expand Up @@ -295,8 +295,9 @@ def createBrokenBuildTask(
res = self.getBrokenBuildTask(title)
if len(res.data) != 0:
self.logger.info(
"Open broken build task (T{}) exists. Skipping creation of a new one."
.format(res.data[0]["id"])
"Open broken build task (T{}) exists. Skipping creation of a new one.".format(
res.data[0]["id"]
)
)
return None

Expand All @@ -313,8 +314,9 @@ def createBrokenBuildTask(
]
)
self.logger.info(
"Response from 'maniphest.edit' creating new task with title '{}': {}"
.format(title, newTask)
"Response from 'maniphest.edit' creating new task with title '{}': {}".format(
title, newTask
)
)
return newTask["object"]

Expand All @@ -334,8 +336,9 @@ def updateRevisionSummary(self, revisionId, summary):
)
else:
self.logger.info(
"Update of revision summary skipped due to deployment environment: '{}'"
.format(self.deployment)
"Update of revision summary skipped due to deployment environment: '{}'".format(
self.deployment
)
)

def get_project_members(self, project_PHID):
Expand Down
25 changes: 12 additions & 13 deletions contrib/buildbot/server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -971,11 +971,9 @@ def handle_build_result(
if not isMaster:
phab.commentOnRevision(
revisionPHID,
(
"(IMPORTANT) The build failed due to an unexpected"
" infrastructure outage. The administrators have been"
" notified to investigate. Sorry for the inconvenience."
),
"(IMPORTANT) The build failed due to an unexpected"
" infrastructure outage. The administrators have been"
" notified to investigate. Sorry for the inconvenience.",
buildName,
)
return SUCCESS, 200
Expand Down Expand Up @@ -1008,8 +1006,9 @@ def handle_build_result(
"{}: Please set your slack username in your Phabricator profile"
" so the landbot can send you direct messages: {}\n{}".format(
authorSlackUsername,
"https://reviews.bitcoinabc.org/people/editprofile/{}"
.format(author["id"]),
"https://reviews.bitcoinabc.org/people/editprofile/{}".format(
author["id"]
),
landBotMessage,
)
)
Expand Down Expand Up @@ -1037,9 +1036,10 @@ def handle_build_result(
)
if updatedTask:
# Only message once all of master is green
(buildFailures, testFailures) = (
tc.getLatestBuildAndTestFailures("BitcoinABC")
)
(
buildFailures,
testFailures,
) = tc.getLatestBuildAndTestFailures("BitcoinABC")
if len(buildFailures) == 0 and len(testFailures) == 0:
if not create_server.db["master_is_green"]:
create_server.db["master_is_green"] = True
Expand Down Expand Up @@ -1166,9 +1166,8 @@ def handle_build_result(
for line in buildLog.splitlines(keepends=True):
logLines.append(line)

msg += (
"Tail of the build log:\n```lines=16,COUNTEREXAMPLE\n{}```"
.format("".join(logLines[-60:]))
msg += "Tail of the build log:\n```lines=16,COUNTEREXAMPLE\n{}```".format(
"".join(logLines[-60:])
)
else:
# Print the failure log for each test
Expand Down
19 changes: 8 additions & 11 deletions contrib/buildbot/test/test_endpoint_status.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,7 @@ def test_status_master_failureAndTaskDoesNotExist_authorDefaultName(self):
"value": (
"[[ {} | build-name ]] is broken on branch"
" 'refs/heads/master'\n\nAssociated"
" commits:\nrABCdeadbeef00000111222333444555666777888000"
.format(
" commits:\nrABCdeadbeef00000111222333444555666777888000".format(
self.teamcity.build_url(
"viewLog.html",
{
Expand Down Expand Up @@ -668,8 +667,7 @@ def test_status_master_failureAndTaskDoesNotExist_authorSlackUsername(self):
"value": (
"[[ {} | build-name ]] is broken on branch"
" 'refs/heads/master'\n\nAssociated"
" commits:\nrABCdeadbeef00000111222333444555666777888000"
.format(
" commits:\nrABCdeadbeef00000111222333444555666777888000".format(
self.teamcity.build_url(
"viewLog.html",
{
Expand Down Expand Up @@ -743,8 +741,7 @@ def test_status_master_failureAndTaskDoesNotExist_scheduledBuild(self):
"value": (
"[[ {} | build-name ]] is broken on branch"
" 'refs/heads/master'\n\nAssociated"
" commits:\nrABCdeadbeef00000111222333444555666777888000"
.format(
" commits:\nrABCdeadbeef00000111222333444555666777888000".format(
self.teamcity.build_url(
"viewLog.html",
{
Expand Down Expand Up @@ -821,8 +818,7 @@ def test_status_master_failureAndTaskDoesNotExist_multipleChanges(self):
"value": (
"[[ {} | build-name ]] is broken on branch"
" 'refs/heads/master'\n\nAssociated"
" commits:\nrABCdeadbeef00000111222333444555666777888000\nrABCdeadbeef00000111222333444555666777888001"
.format(
" commits:\nrABCdeadbeef00000111222333444555666777888000\nrABCdeadbeef00000111222333444555666777888001".format(
self.teamcity.build_url(
"viewLog.html",
{
Expand Down Expand Up @@ -1109,8 +1105,7 @@ def test_status_revision_buildFailed(self):
"type": "comment",
"value": (
"(IMPORTANT) Build [[{} | build-name (linux)]] failed.\n\nTail"
" of the build log:\n```lines=16,COUNTEREXAMPLE\ndummy log```"
.format(
" of the build log:\n```lines=16,COUNTEREXAMPLE\ndummy log```".format(
self.teamcity.build_url(
"viewLog.html",
{
Expand Down Expand Up @@ -1702,7 +1697,9 @@ def build_line(
color=(
"lightgrey"
if status == BuildStatus.Unknown
else "brightgreen" if status == BuildStatus.Success else "red"
else "brightgreen"
if status == BuildStatus.Success
else "red"
),
)
return '| [[{} | {}]] | {{image uri="{}", alt="{}"}} |\n'.format(
Expand Down
5 changes: 3 additions & 2 deletions contrib/buildbot/testutil.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ class AnyWith(cls):
def __eq__(self, other):
if not isinstance(other, cls):
raise AssertionError(
"Argument class type did not match.\nExpected:\n{}\n\nActual:\n{}"
.format(pformat(cls), pformat(other))
"Argument class type did not match.\nExpected:\n{}\n\nActual:\n{}".format(
pformat(cls), pformat(other)
)
)
if attrs is not None:
for attr, expectedValue in attrs.items():
Expand Down
12 changes: 6 additions & 6 deletions contrib/devtools/chainparams/test_make_chainparams.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,11 @@ def __init__(
def checkFailCounter(self):
self.failCounter -= 1
if self.failCounter < 0:
raise Exception("""error code: -99
raise Exception(
"""error code: -99
error message:
mock error""")
mock error"""
)

def getblockchaininfo(self):
self.checkFailCounter()
Expand Down Expand Up @@ -285,10 +287,8 @@ def test_wrong_chain(self):
CheckMockFailure(
self,
args,
(
"expected was 'getblockheader"
" 0000000000000000003ef673ae12bc6017481830d37b9c52ce1e79c080e812b8'"
),
"expected was 'getblockheader"
" 0000000000000000003ef673ae12bc6017481830d37b9c52ce1e79c080e812b8'",
)

def test_bitcoin_cli_failures_testnet(self):
Expand Down
9 changes: 5 additions & 4 deletions contrib/devtools/test-security-check.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,16 @@

def write_testcode(filename):
with open(filename, "w", encoding="utf8") as f:
f.write("""
f.write(
"""
#include <stdio.h>
int main()
{
printf("the quick brown fox jumps over the lazy god\\n");
return 0;
}
""")
"""
)


def clean_files(source, executable):
Expand Down Expand Up @@ -328,8 +330,7 @@ def test_PE(self):
),
(
1,
executable
+ ": failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION"
executable + ": failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION"
" CONTROL_FLOW",
),
)
Expand Down
Loading

0 comments on commit a5d12e2

Please sign in to comment.