GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-110558: Run ruff on Argument Clinic in CI by AlexWaygood · Pull Request #110559 · python/cpython

Conversation

@AlexWaygood

@AlexWaygood AlexWaygood commented Oct 9, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Run ruff on Argument Clinic in CI #110558

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@AlexWaygood
pythongh-110558: Run ruff on Argument Clinic in CI
dec89bc
@AlexWaygood
AlexWaygood requested review from erlend-aasland and hugovk as code owners October 9, 2023 12:18
@bedevere-app bedevere-app Bot added the awaiting review label Oct 9, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Oct 9, 2023
Run ruff on Argument Clinic in CI #110558
Closed
@AlexWaygood AlexWaygood added the skip news label Oct 9, 2023
@AlexWaygood
Fix pyflakes complaints
b52e2d4
@AlexWaygood

AlexWaygood commented Oct 9, 2023

Copy link
Copy Markdown
Member Author

https://github.com/python/cpython/actions/runs/6456495002/job/17526096001?pr=110559 shows what it looks like when the job fails

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood
also fix a ruff deprecation warning regarding an environment variable…
ead7587
…, while we're here
@AlexWaygood
AlexWaygood requested a review from ezio-melotti as a code owner October 9, 2023 12:22
erlend-aasland
erlend-aasland reviewed Oct 9, 2023
View reviewed changes
Comment thread Tools/clinic/.ruff.toml Resolved

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood and others added 2 commits October 9, 2023 14:24
@AlexWaygood
oops i deleted the wrong variable
75c7a56
@AlexWaygood
comments
485c548
@AlexWaygood

AlexWaygood commented Oct 9, 2023

Copy link
Copy Markdown
Member Author

A few questions:

  1. 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.)
  2. 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.
  3. Any other rules we want to add right off the bat? (We can always add more in follow-up PRs.)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Oct 9, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author
  1. 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
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland
erlend-aasland approved these changes Oct 9, 2023
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Oct 9, 2023
@erlend-aasland

erlend-aasland commented Oct 9, 2023

Copy link
Copy Markdown
Contributor
  1. 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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood
Also run all the pyflakes checks on Lib/test/test_clinic.py
5850844
@AlexWaygood AlexWaygood added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 10, 2023
hugovk
hugovk approved these changes Oct 10, 2023
View reviewed changes
@AlexWaygood

AlexWaygood commented Oct 10, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood
AlexWaygood enabled auto-merge (squash) October 10, 2023 07:38
@AlexWaygood
AlexWaygood merged commit 7b2764e into python:main Oct 10, 2023
@AlexWaygood
AlexWaygood deleted the ruff-arg-clinic branch October 10, 2023 07:52
@miss-islington

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting core review label Oct 10, 2023
@miss-islington

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington miss-islington assigned AlexWaygood Oct 10, 2023
@miss-islington

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app

bedevere-app Bot commented Oct 10, 2023

Copy link
Copy Markdown

GH-110598 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 10, 2023
AlexWaygood added a commit to AlexWaygood/cpython that referenced this pull request Oct 10, 2023
@AlexWaygood
[3.12] pythongh-110558: Run ruff on Argument Clinic in CI (python#110559
da73e4c
)
@AlexWaygood

AlexWaygood commented Oct 10, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood AlexWaygood removed the needs backport to 3.11 only security fixes label Oct 10, 2023
@AlexWaygood AlexWaygood mentioned this pull request Oct 10, 2023
gh-110558: Enable ruff's pyupgrade rules when running on Argument Clinic #110603
Merged
hugovk pushed a commit that referenced this pull request Oct 10, 2023
@AlexWaygood
[3.12] gh-110558: Run ruff on Argument Clinic in CI (#110559) (#110598)
3688672
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@AlexWaygood @Glyphack
pythongh-110558: Run ruff on Argument Clinic in CI (python#110559)
a3db2eb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk approved these changes
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@ezio-melotti 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.

Assignees

@AlexWaygood AlexWaygood

Labels

skip news

Projects

None yet

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.

4 participants

@AlexWaygood @erlend-aasland @miss-islington @hugovk
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.