Skip to content

[py] Lint and format all python files #15828

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 30, 2025
Merged

[py] Lint and format all python files #15828

merged 1 commit into from
May 30, 2025

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented May 30, 2025 •
edited by qodo-merge-pro bot
Loading

User description

💥 What does this PR do?

This PR adjusts the linting/formatting (ruff) settings in ./py/tox.ini so it scans the entire ./py directory tree, not just specific directories. Previously, it was skipping files in the directory root. This brings tox in line with how we do linting in CI via bazel.

🔄 Types of changes

  • internal/build infrastructure

PR Type

Enhancement

Description

  • Update ruff linting to cover all Python files

  • Change ruff commands to scan entire ./py directory

  • Align tox linting with CI (bazel) behavior

Changes walkthrough 📝

Relevant files
Configuration changes
tox.ini
Update tox ruff commands to lint all Python files               

py/tox.ini

  • Modified ruff lint and format commands to target all files in the
    directory
  • Replaced specific file/directory targets with a dot (.)
  • Ensures no Python files are skipped during linting/formatting
    		• +2/-2     
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @selenium-ci selenium-ci added the C-py Python Bindings label May 30, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented May 30, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented May 30, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add exclusions to linting

    Using . without exclusions will lint and format all Python files in the
    directory, including potentially unwanted files like virtual environments, build
    artifacts, or third-party code. Consider adding exclusions or specifying target
    directories.

    py/tox.ini [40-41]

    -ruff check --fix --show-fixes --exit-non-zero-on-fix .
-ruff format --exit-non-zero-on-format .
+ruff check --fix --show-fixes --exit-non-zero-on-fix . --exclude .venv/,build/,dist/
+ruff format --exit-non-zero-on-format . --exclude .venv/,build/,dist/
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: Valid concern about using . without exclusions potentially linting unwanted directories like virtual environments or build artifacts. This is a reasonable best practice improvement for performance and cleanliness.

    		Low
    • More

    @cgoldberg cgoldberg merged commit 3788f49 into trunk May 30, 2025
    12 checks passed
    @cgoldberg cgoldberg deleted the py-tox-lint-everything branch May 30, 2025 13:29
    @cgoldberg cgoldberg mentioned this pull request May 30, 2025
    Merged
    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
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    C-py Python Bindings Review effort 1/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @cgoldberg @selenium-ci