-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
Update pre-commit hooks #137591
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
Update pre-commit hooks #137591
Conversation
Let's update the other We're on Ruff v0.11.8. Ruff 0.12.* gives out a lot of syntax errors, for example: "invalid-syntax: Cannot use If you like we can leave those for another time and just bump to to the latest 0.11: v0.11.13. |
I fixed this by specifying the target Python version. |
.pre-commit-config.yaml
Outdated
hooks: | ||
- id: ruff | ||
name: Run Ruff (lint) on Doc/ | ||
args: [--exit-non-zero-on-fix] | ||
files: ^Doc/ | ||
- id: ruff | ||
name: Run Ruff (lint) on Lib/test/ | ||
args: [--exit-non-zero-on-fix] | ||
args: [--exit-non-zero-on-fix, --target-version=py313] |
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.
I guess a low version was initially chosen to make backports easier?
Anyway, let's move the config to a .ruff.toml
file so it also applies if running Ruff directly.
args: [--exit-non-zero-on-fix, --target-version=py313] | |
args: [--exit-non-zero-on-fix] |
We have a top-level that defines target-version = "py310"
.
Let's override that in Lib/test/.ruff.toml
, either:
target-version = "py313"
like inDoc/.ruff.toml
[per-file-target-version]
like inTools/build/.ruff.toml
(might be too files many to be worth it)
And we can also remove the t-string files from extend-exclude
and put those under [per-file-target-version]
for py314
.
(Could do the same for test_grammar.py
but it has some F811 errors that would need deciding about.)
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.
Done.
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.
Thank you!
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.
We can be a little more conservative here:
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Co-authored-by: Adam Turner <[email protected]>
I have made the requested changes; please review again. |
Thanks for making the requested changes! @AA-Turner, @hugovk: please review the changes made to this pull request. |
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.
Cheers
Thanks @mhsmith for the PR, and @AA-Turner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
(cherry picked from commit b36d23f) Co-authored-by: Malcolm Smith <[email protected]> Co-authored-by: Adam Turner <[email protected]>
Sorry, @mhsmith and @AA-Turner, I could not cleanly backport this to
|
GH-137621 is a backport of this pull request to the 3.14 branch. |
Co-authored-by: Malcolm Smith <[email protected]> Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Adam Turner <[email protected]> (cherry picked from commit b36d23f)
GH-137641 is a backport of this pull request to the 3.14 branch. |
Split from #137186. That PR no longer needs the zizmor update, but since I've worked out exactly how to do it, we might as well merge it anyway.