lint: Convert Python linter to Python #24794
Conversation
|
Concept/approach ACK |
|
I think it's slightly weird that |
| import sys | ||
|
|
||
| DEPS = ['flake8', 'mypy', 'pyzmq'] | ||
| MYPY_CACHE_DIR = f"{os.getenv('BASE_ROOT_DIR', '')}/test/.mypy_cache" |
There was a problem hiding this comment.
Unrelated: If run outside the CI runner, this will likely result in /test/.mypy_cache, which may not be wanted?
There was a problem hiding this comment.
Yes, I noticed this as well but it is the existing behavior. I used getenv with the empty fallback because I wanted to potentially change it to a better one but forgot to think about it. What would you suggest as a better fallback?
|
I was half hoping we could just call the flake8 api directly, and check our files, rather than just moving to wrapping subprocess invocations of flake8 in python instead of bash. Maybe something for the future. |
This sounds neat for a follow-up, but personally at least I think as a first step calling out to tools like |
65e36e1 to
28e7af4
Compare
I am slightly unsure if you would like me to make a change or not, but I think not. Otherwise, let me know :) I was thinking about this quite a bit when initially putting it on the list but after seeing the errors produced when |
|
Addressed review comments. |
28e7af4 to
47b66ac
Compare
|
Tested ACK 47b66ac |
…n.py` e245c5c doc: Fix a link to `test/lint/lint-python.py` (Hennadii Stepanov) Pull request description: This PR is a follow up to bitcoin/bitcoin#24794. Closes #588. Top commit has no ACKs. Tree-SHA512: 9305705082c5e8f0c093506b4931a13b50e33e8315f6758ee525bc7f6d840b517af5d1092cee4bcc1bfc553d629b771f1893f27d0f514639c2da295bb604877a
e245c5c doc: Fix a link to `test/lint/lint-python.py` (Hennadii Stepanov) Pull request description: This PR is a follow up to bitcoin#24794. Closes bitcoin-core/gui#588. Top commit has no ACKs. Tree-SHA512: 9305705082c5e8f0c093506b4931a13b50e33e8315f6758ee525bc7f6d840b517af5d1092cee4bcc1bfc553d629b771f1893f27d0f514639c2da295bb604877a
The outputs provided by the Python version should be exactly the same as the ones from the shell version.
There is small improvement here: Previously only the dependency of
flake9was checked, now all dependencies are checked before running.I also tried to mostly follow the recommendations here but happy to make more changes if there is still room for improvement.