GeezerVison is showing github.com — gh-110558: Run ruff on Argument Clinic in CI by AlexWaygood · Pull Request #110559 · python/cpython
Conversation
AlexWaygood
commented
Oct 9, 2023
•
edited by bedevere-app
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
A few questions:
- Should we also run all pyflakes checks on
Lib/test/test_clinic.py? I'd quite like to; we also had things like unused variables etc. in the test file in the past. If so, maybe we should remove that file from the existing ruff check? (But having it manually excluded from the existing ruff check would add complexity, and ruff is incredibly fast, so maybe it's okay to run ruff twice in CI on that one file.)
- Do we want to add
ruff to Tools/requirements-dev.txt? It might be helpful for running ruff locally -- you could just run pip install -r Tools/requirements-dev.txt in a venv to install all the linting tools for Argument Clinic. On the other hand, we're running ruff via pre-commit in CI, and it's really easy to do that locally as well, so maybe it's not necessary to add it to Tools/requirements-dev.txt.
- Any other rules we want to add right off the bat? (We can always add more in follow-up PRs.)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
AlexWaygood
commented
Oct 9, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
- Should we also run all pyflakes checks on
Lib/test/test_clinic.py? I'd quite like to; we also had things like unused variables etc. in the test file in the past. If so, maybe we should remove that file from the existing ruff check? (But having it manually excluded from the existing ruff check would add complexity, and ruff is incredibly fast, so maybe it's okay to run ruff twice in CI on that one file.)
On the one hand, a nicer way of solving this might be to work on slowly adding more enabled error codes to https://github.com/python/cpython/blob/main/Lib/test/.ruff.toml. On the other hand, there are currently 3095 errors reported by ruff if you enable all the pyflakes rules and run it on Lib/test/, whereas there are 0 errors reported by ruff if you do the same thing, but run it only on Lib/test/test_clinic.py. So it would probably be quite a while before we'd be able to enable all the pyflakes checks on all of Lib/test/...
|
erlend-aasland reacted with laugh emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- Do we want to add
ruff to Tools/requirements-dev.txt? It might be helpful for running ruff locally -- you could just run pip install -r Tools/requirements-dev.txt in a venv to install all the linting tools for Argument Clinic. On the other hand, we're running ruff via pre-commit in CI, and it's really easy to do that locally as well, so maybe it's not necessary to add it to Tools/requirements-dev.txt.
Running ruff via pre-commit sounds good to me; having more such stuff in pre-commit is convenient and useful.
|
AlexWaygood reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
AlexWaygood
commented
Oct 10, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
I'm going to try backporting this, but if ruff fails for any reason on the older branches, I'm not going to try too hard to fix it (I'll just close the PRs).
|
hugovk reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Sorry, @AlexWaygood, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 7b2764e798e400b8f5fcc199739405e6fbd05c20 3.12
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Sorry, @AlexWaygood, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 7b2764e798e400b8f5fcc199739405e6fbd05c20 3.11
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
AlexWaygood
commented
Oct 10, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
I created a backport PR to 3.12, but there's a ton of ruff failures on 3.11, so I won't bother with that one.
|
erlend-aasland reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
hugovk
hugovk approved these changes
ezio-melotti
Awaiting requested review from ezio-melotti
ezio-melotti is a code owner
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|