Skip to content

Conversation

@nschonni
Copy link
Member

This workflow really only checks for python issues, so renamed the file and restricted it to only run when the files are impacted.

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: 3.7
Copy link
Member Author

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

Copy link
Contributor

@cclauss cclauss Apr 12, 2023

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: The python-version input is not set. The version of Python currently in PATH 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

@cclauss
Copy link
Contributor

cclauss commented Apr 11, 2023

Let's instead replace flake8 with ruff (written in Rust) as we have done on node-gyp, gyp-next, tap2junit.

- .github/workflows/lint-python.yml
pull_request:
paths:
- '**/*.py'
Copy link
Contributor

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this repo?

@nschonni
Copy link
Member Author

Closing since the ruff PR landed upstream, so it likely makes more sense to use the other PR

@nschonni nschonni closed this Apr 14, 2023
@nschonni nschonni deleted the pylint-filter branch April 14, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants