Skip to content

[py] Lint Python with ruff#15811

Merged
p0deje merged 1 commit intotrunkfrom
ruff-lint
May 29, 2025
Merged

[py] Lint Python with ruff#15811
p0deje merged 1 commit intotrunkfrom
ruff-lint

Conversation

@p0deje
Copy link
Member

@p0deje p0deje commented May 28, 2025

Use Ruff directly to format and lint Python source code files.

@p0deje p0deje requested a review from cgoldberg May 28, 2025 23:59
@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels May 28, 2025
cgoldberg
cgoldberg reviewed May 29, 2025
View reviewed changes
Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

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

Do you need to add --exit-non-zero-on-fix to check, and --exit-non-zero-on-format to format? We want the CI job to fail if it hits any violations.

Other than that, LGTM

@p0deje
Copy link
Member Author

p0deje commented May 29, 2025

Do you need to add --exit-non-zero-on-fix to check, and --exit-non-zero-on-format to format

Our CI job runs the format and then checks for git diff, so it will fail.

@cgoldberg cgoldberg mentioned this pull request May 29, 2025
Closed
@cgoldberg
Copy link
Member

cgoldberg commented May 29, 2025

@p0deje There are some issues that ruff won't auto-fix (like lines that are too long)... so git diff wouldn't see those. You might want to add those args to make sure it fails.

@p0deje
Copy link
Member Author

p0deje commented May 29, 2025

@cgoldberg Hmm, I assumed that when Ruff cannot fix/format, it would fail with non-zero exit code. At least this is how I read the flag name. Will update.

@cgoldberg
Copy link
Member

cgoldberg commented May 29, 2025

I'm not 100% sure what the exit code is when it can't fix... I'd have to try, but I suppose it doesn't hurt to add the flags.

@p0deje
Copy link
Member Author

p0deje commented May 29, 2025

I did some testing and ruff seems to exit non-zero when it cannot autofix:

$ ruff check --fix scripts/pinned_browsers.py; echo $status
scripts/pinned_browsers.py:208:5: E741 Ambiguous variable name: `l`
    |
206 |     mac = None
207 |     mac_hash = None
208 |     l = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA…
    |     ^ E741
209 |
210 |     for data in all_data:
    |

scripts/pinned_browsers.py:208:5: F841 Local variable `l` is assigned to but never used
    |
206 |     mac = None
207 |     mac_hash = None
208 |     l = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA…
    |     ^ F841
209 |
210 |     for data in all_data:
    |
    = help: Remove assignment to unused variable `l`

Found 2 errors.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
1

For format, there seem to be no cases when it would exit non-zero and not format files.

I am going to merge this as-is, we should be safe even w/o the flags.

@p0deje p0deje merged commit 001b95b into trunk May 29, 2025
21 checks passed
@p0deje p0deje deleted the ruff-lint branch May 29, 2025 02:49
This was referenced Sep 12, 2025
Closed
Closed
Merged
This was referenced Sep 19, 2025
Closed
Closed
Closed
This was referenced Oct 1, 2025
Closed
Closed
Closed
@dependabot dependabot bot mentioned this pull request Oct 24, 2025
Closed
This was referenced Dec 21, 2025
Open
Closed
Closed
Closed
This was referenced Jan 19, 2026
Open
Open
This was referenced Jan 19, 2026
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgoldberg cgoldberg cgoldberg approved these changes

Assignees

No one assigned

Labels

B-build Includes scripting, bazel and CI integrations C-py Python Bindings

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@p0deje @cgoldberg @selenium-ci