Skip to content

Add pre-commit config for pyflakes #28

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

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

honno
Copy link
Member

@honno honno commented Oct 14, 2021

I keep on forgetting to run pyflakes before pushing, so I thought I'd use pre-commit. Here's a minimal config for the tool. As there doesn't seem to be a pre-commit hook for pyflakes itself, I just use flake8 and run only the pyflake-related checks via flake8 --select F. I've also updated the linting workflow to use pre-commit, to keep consistency (although I doubt flake8 versions will change the pyflake linting behaviour).

It is "another thing", so I understand if you're not interested in merging. For the future I was thinking it might be nice to create a custom pre-commit hook that ensured any staged changes to ./generate_stubs.py would trigger a check to see the related files have been updated.

@@ -0,0 +1,6 @@
repos:
- repo: https://github.com/pycqa/flake8
rev: '4.0.1'
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep this unpinned. Whenever pyflakes is updated it may create new failures, but those are also things we'd want to know about. Also we are going to want to add 3.10 testing pretty soon, and I don't know if this versions supports it yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I had just used the latest release tag (didn't think to unpin it, makes sense). The 4.0 release was tested for py3.10 it looks like at least.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any way to remove this? I tried removing it but that causes it to fail.

@asmeurer
Copy link
Member

This is fine. It seems optional.

We can try generate stubs too. Sometimes, I do manually update the stubs from a PR to the API repo so that I don't have to wait for it to get merged. Also the API repo is going to change to rst pretty soon, which will break generate stubs until it gets updated.

@asmeurer asmeurer merged commit cbc2b33 into data-apis:master Oct 14, 2021
@honno honno deleted the pyflakes-pre-commit branch October 15, 2021 09:59
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