lint: Convert lint-circular-dependencies.sh to Python #24915
Conversation
|
I think you could squash the commits since they're related. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, 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. |
It's done @brunoerg 👍 |
|
Tested 23e0476. The new version could maybe use a line break before the As a test I added:
Before ( After ( Sidenote: I'm not sure why adding a circular dependency causes an existing circular dependency to disappear |
|
@KevinMusgrave Indeed, the newline after each message from the original script was missing. |
|
Tested ACK ca52afd Output: |
| dependencies_output = subprocess.run( | ||
| command, | ||
| stdout=subprocess.PIPE, |
There was a problem hiding this comment.
any reason to ignore errors?
| dependencies_output = subprocess.run( | |
| command, | |
| stdout=subprocess.PIPE, | |
| dependencies_output = subprocess.check_output( | |
| command, |
There was a problem hiding this comment.
Because ../contrib/devtools/circular-dependencies.py does not exit with 0 if there is at least one circular dependency, resulting in subprocess.check raising an error
|
Tested ACK 79635c7 |
|
|
||
| # Using glob.glob since subprocess.run's globbing won't work without shell=True | ||
| files = [] | ||
| for path in ["*", "*/*", "*/*/*"]: |
There was a problem hiding this comment.
Wouldn't it be better to call git ls-files -- '*.h' '*.cpp'?
| for extension in ["h", "cpp"]: | ||
| files.extend(glob.glob(f"{path}.{extension}")) | ||
|
|
||
| command = ["python3", "../contrib/devtools/circular-dependencies.py", *files] |
There was a problem hiding this comment.
Wouldn't it be better to use sys.executable instead of "python3"?
…ython 79635c7 lint: Convert lint-circular-dependencies.sh to Python (Smlep) Pull request description: Here is a port of `/test/lint/lint-circular-dependencies.sh` to a Python-script as part of the request of bitcoin#24783. It aims to provide the same output as the bash version. ACKs for top commit: laanwj: Tested ACK 79635c7 Tree-SHA512: f18077018f1229dd933cfe2bf0cfe7dc7d6538961c96a83c7a1f05e0cec4b068ca05502d68410d2aa4b6864523424db386e38233735190525904c2a8e9d2ba13
Here is a port of
/test/lint/lint-circular-dependencies.shto a Python-script as part of the request of #24783.It aims to provide the same output as the bash version.