Skip to content

[Backport branch-perf-v15] improvement(treewide): Catch-up on all ruff related PRs #11293

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

Draft
wants to merge 6 commits into
base: branch-perf-v15
Choose a base branch
from

Conversation

pehala
Copy link
Contributor

@pehala pehala commented Jun 29, 2025

V15 branch has ruff, but it was outdated on actual formatting and rules. Cherrypick neccessary commits to get it up to date. Prerequisite for #11291

Testing

  • Unit tests

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

vponomaryov and others added 6 commits June 29, 2025 09:27
Enable following rules:
- F821, "undefined-name" -> Undefined name {name}. {tip}
- F823, "undefined-local" -> Local variable {name} referenced before assignment

(cherry picked from commit 30aee48)
since we have this check on older branches using pylint
and it's easily fixable checked, we are introducing this one
to minimize issues with backporting

Ref: https://docs.astral.sh/ruff/rules/f-string-missing-placeholders/#f-string-missing-placeholders-f541

(cherry picked from commit 4017a77)
those lint rule helps removing unneed code parts

Ref: https://docs.astral.sh/ruff/rules/#flake8-pie-pie

(cherry picked from commit 573abee)
we shouldn't let those get into the the code without
a very good reason (like memoize/cache)

Ref: https://docs.astral.sh/ruff/rules/mutable-argument-default/
(cherry picked from commit 2686304)
raise errors where it should be raised but don't

(cherry picked from commit c22fba4)
@pehala pehala added the New Hydra Version PR# introduces new Hydra version label Jun 29, 2025
@pehala pehala marked this pull request as draft June 29, 2025 07:46
@@ -27,7 +27,7 @@ docker==7.1.0
python-jenkins==1.7.0
ssh2-python3==1.11.1
parameterized==0.8.1
ruff==0.4.7 # Needed for pre-commit hooks
ruff==0.11.13 # Needed for pre-commit hooks
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to build the image for it to work

Copy link
Contributor

Choose a reason for hiding this comment

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

New Hydra Version won't work on PRs to branches...

Copy link
Contributor

Choose a reason for hiding this comment

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

or at least it won't work, cause the pipeline doesn't exist in this branch....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there an automated way of doing it, so I dont need permissions for docker hub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, should we backport the pipeline here (and other branches)?

Copy link
Contributor

Choose a reason for hiding this comment

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

the automated way is the pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Hydra Version PR# introduces new Hydra version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants