-
Notifications
You must be signed in to change notification settings - Fork 166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: restrict Python linting runs #3301
Conversation
uses: actions/setup-python@v4 | ||
with: | ||
python-version: 3.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cclauss I didn't think the Python need to be pinned for the linting job, but correct me if i'm wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the warnings that appear under the twisty for this GitHub Action job step:
Run actions/setup-python@v4
Warning: Neither 'python-version' nor 'python-version-file' inputs were supplied. Attempting to find '.python-version' file.
Warning: .python-version doesn't exist.
Warning: Thepython-version
input is not set. The version of Python currently inPATH
will be used.
This ended up with Python 3.8. Explicit is better than implicit (The Zen of Python)
Flake8 changes based on the Python that it is run on. https://flake8.pycqa.org
Note
It is very important to install Flake8 on the correct version of Python for your needs. If you want Flake8 to properly parse new language features in Python 3.5 (for example), you need it to be installed on 3.5 for Flake8 to understand those features. In many ways, Flake8 is tied to the version of Python on which it runs.
Ruff does not run on Python so it changes in similar ways but only based on --target-version=pyXX
Let's instead replace |
pull_request: | ||
paths: | ||
- '**/*.py' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Node.js has Python files that do not have a .py suffix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this repo?
Closing since the ruff PR landed upstream, so it likely makes more sense to use the other PR |
This workflow really only checks for python issues, so renamed the file and restricted it to only run when the files are impacted.