Skip to content

Conversation

@ulgens
Copy link

@ulgens ulgens commented Dec 26, 2025

Description

I was checking the lint setup and realized running pre-commit locally on latest main doesn't produce a clean result. I checked why this wasn't the case on CI and found out that pre-commit ruff and CI ruff use slightly different configuration. I tried to centralize that config, and found out there is a bug with ruff-pre-commit but still DRY'ed the ruff calls.

Considering the removal of the "include" setting causes only a single change in a single file, I think that settings can be removed compeletely but I didn't want to change configuration without asking about it first.

Issues

Reminders

@ulgens ulgens requested a review from a team as a code owner December 26, 2025 12:54
Comment on lines +928 to 929
pre-commit run --all-files
mypy sentry_sdk
Copy link

Choose a reason for hiding this comment

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

Bug: The ruff-format pre-commit hook is missing the --check argument, causing the linters tox environment to modify files in-place instead of only checking for formatting issues.
Severity: HIGH | Confidence: High

🔍 Detailed Analysis

The linters tox environment was changed to use pre-commit run --all-files instead of a direct ruff format --check command. However, the ruff-format hook in .pre-commit-config.yaml is missing the args: [--check] configuration. As a result, running tox -e linters no longer just checks for formatting issues but actively reformats files in-place. This is a functional regression, as a linting environment is expected to be a read-only check, especially in CI, and should not modify source files.

💡 Suggested Fix

Add args: [--check] to the ruff-format hook in the .pre-commit-config.yaml file to restore the previous check-only behavior and prevent file modifications during linting.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: tox.ini#L928-L929

Potential issue: The `linters` tox environment was changed to use `pre-commit run
--all-files` instead of a direct `ruff format --check` command. However, the
`ruff-format` hook in `.pre-commit-config.yaml` is missing the `args: [--check]`
configuration. As a result, running `tox -e linters` no longer just checks for
formatting issues but actively reformats files in-place. This is a functional
regression, as a linting environment is expected to be a read-only check, especially in
CI, and should not modify source files.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7943798

Copy link
Author

Choose a reason for hiding this comment

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

I believe this is okay. Explicitly saying that files shouldn't be changed doesn't yield a benefit on CI.

@ulgens ulgens changed the title fix: DRY ruff calls ci: DRY ruff calls Dec 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant