-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add flake8 to pre-commit checks, with local bugbear as a plug-in #425
Conversation
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.
Just want to get some confirmations and see if we can clean this up more ... Let's also see how CI goes ...
@@ -13,3 +13,11 @@ repos: | |||
- id: black | |||
args: | |||
- --preview | |||
|
|||
- repo: https://github.com/pycqa/flake8 | |||
rev: 6.1.0 |
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.
What does this rev refer to? Our latest tag / release is 23.9.16
If we can not use it cause re remove it or make it 99.0.0 or something + comment why we've done so
hooks: | ||
- id: flake8 | ||
args: [--append-config=.flake8-pre-commit.cfg] | ||
additional_dependencies: [attrs>=19.2.0] |
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.
Why don't we need flake8 here? Can't our pyproject.toml handle this to avoid copy pasta / more things to change in the future?
- id: flake8 | ||
args: [--append-config=.flake8-pre-commit.cfg] | ||
additional_dependencies: [attrs>=19.2.0] | ||
exclude: ^tests\/ |
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.
Just so I understand, this is so we don't hit the test code that htis our checks on purpose right?
- id: flake8 | ||
args: [--append-config=.flake8-pre-commit.cfg] | ||
additional_dependencies: [attrs>=19.2.0] | ||
exclude: ^tests\/ |
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.
Just so I understand, this is so we don't hit the test code that htis our checks on purpose right?
O, my bad, this is for external users and not for running on CI runs? I'm super lost ... (can you tell I don't use pre-commit) I tried to re-run the issue and PR, but still don't get what this solves / can be used for ... so if that could be explained that would be great too ... sorry, but no GitHub / precommit power user dur to working at a $bigCompany |
Hey, sorry if I didn't explain this very well! Yeah the goal is to add a So I was trying to add that flake8 check for this repo on pre-commit. The tricky part is, that since it's a self-contained flake8 installation - how do we also run checks defined in our local plugin? We can specify So the approach I came up with was to specify for pre-commit to use its own installation of flake8, and and pass in an arg for that additional config file so that it would pick up the bugbear plugin py file from within the repo. It's a little hacky, and I'm still troubleshooting it myself - seems to hang indefinitely on my PC if I run on all files. Started a draft because I kinda wanted to see if it worked in CI. Seemed to get picked up properly in the pre-commit CI at least. |
So, is your goal to just run flake8-bugbear via precommit? We do this with a project: https://github.com/pypa/bandersnatch/blob/main/.pre-commit-config.yaml#L40 Isn't that what this does? What's the missing example with this approach I'm still not following?
|
The goal was to run flake8-bugbear (the live one) on this project itself, so that new rules would apply in real-time |
Something wonky about this config, probably more trouble than it's worth. Adding the basic checks here instead #427 |
For #412
Considered doing this as a repo local-hook but it would require somehow hardcoding the venv location.
Implemented using pre-commit instance of flake8, and passing some additional config (and attr dependency) so that it can use the live bugbear.py from the repo.
This had to be split out to a separate config, because if it was in the base one, it would break local venv usage of flake8 because there would be duplicate bugbear plugins.