Skip to content

Conversation

@cgoldberg
Copy link
Member

💥 What does this PR do?

Autofix linting errors, but still fail if encountered.

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)
  • New feature (non-breaking change which adds functionality and tests!)
  • Breaking change (fix or feature that would cause existing functionality to change)

@qodo-code-review
Copy link
Contributor

PR Type

Enhancement


Description

  • Add automatic linting error fixes with ruff

  • Maintain failure on detected issues for CI

  • Show which fixes were applied during check


File Walkthrough

Relevant files
Enhancement
ruff_check.py
Enable automatic linting fixes with failure reporting       

py/private/ruff_check.py

  • Added --fix flag to automatically fix linting errors
  • Added --show-fixes flag to display applied fixes
  • Added --exit-non-zero-on-fix flag to fail if fixes were needed
  • Maintains CI failure behavior while enabling automatic corrections
+1/-1     

@selenium-ci selenium-ci added the C-py Python Bindings label Feb 2, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Feb 2, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Feb 2, 2026

PR Code Suggestions ✨

Latest suggestions up to 6577d43

CategorySuggestion                                                                                                                                    Impact
Possible issue
Force exclude patterns to apply

Add the --force-exclude flag to the ruff command to ensure that exclude patterns
are always honored, even for explicitly provided file paths.

py/private/ruff_check.py [36]

-cmd = [ruff, "check", "--fix", "--show-fixes", "--exit-non-zero-on-fix", "--config=py/pyproject.toml"]
+cmd = [ruff, "check", "--fix", "--show-fixes", "--exit-non-zero-on-fix", "--force-exclude", "--config=py/pyproject.toml"]
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that ruff ignores exclude patterns for explicitly passed files and proposes using --force-exclude to ensure the script's exclusion logic is always respected, which improves its robustness.

Low
Learned
best practice
Make lint read-only by default

Make the default :lint task read-only by explicitly disabling fixes, and add a
separate opt-in task for applying fixes to avoid mutating working trees
unexpectedly.

rake_tasks/python.rake [180]

-Bazel.execute('run', [], '//py:ruff-check')
+Bazel.execute('run', %w[-- --no-fix], '//py:ruff-check')
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Avoid side-effecting “lint” tasks by default; make autofix opt-in to keep automation rerunnable and noise-free.

Low
  • More

Previous suggestions

Suggestions up to commit 1abcc21
CategorySuggestion                                                                                                                                    Impact
General
Handle missing ruff executable

Add a try/except FileNotFoundError block around the subprocess.run call to
handle cases where the ruff executable is not found.

py/private/ruff_check.py [37]

-return subprocess.run(cmd + exclude_args + dirs + extra_args).returncode
+try:
+    result = subprocess.run(cmd + exclude_args + dirs + extra_args)
+    return result.returncode
+except FileNotFoundError:
+    print(f"Error: ruff binary not found at {ruff}", file=sys.stderr)
+    return 1
Suggestion importance[1-10]: 7

__

Why: This suggestion correctly identifies a potential FileNotFoundError if the ruff binary is missing and provides a robust solution to handle it gracefully, improving the script's error handling and user experience.

Medium
Remove redundant flags from ruff command

Remove the redundant --exit-non-zero-on-fix and --show-fixes flags from the ruff
command, as their behavior is either default or unnecessary in this context.

py/private/ruff_check.py [34-37]

 def run_check(ruff, exclude_args, dirs, extra_args):
     """Run ruff check (linting)."""
-    cmd = [ruff, "check", "--fix", "--show-fixes", "--exit-non-zero-on-fix", "--config=py/pyproject.toml"]
+    cmd = [ruff, "check", "--fix", "--config=py/pyproject.toml"]
     return subprocess.run(cmd + exclude_args + dirs + extra_args).returncode
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that the --exit-non-zero-on-fix flag is deprecated and redundant, improving the command by removing it. Removing --show-fixes is a reasonable simplification, making the command cleaner.

Low

@cgoldberg cgoldberg merged commit 492d462 into SeleniumHQ:trunk Feb 2, 2026
29 checks passed
@cgoldberg cgoldberg deleted the py-lint-fix branch February 2, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants