Skip to content

Chore: Pin ruff rule selection - #175

Merged
ModeSevenIndustrialSolutions merged 1 commit into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:chore/pin-ruff-rule-selection
Jul 30, 2026
Merged

Chore: Pin ruff rule selection#175
ModeSevenIndustrialSolutions merged 1 commit into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:chore/pin-ruff-rule-selection

Conversation

@ModeSevenIndustrialSolutions

Copy link
Copy Markdown
Contributor

Why

This repository runs the ruff hook with no ruff configuration, so it applies whatever ruff's default rule set happens to be. That default is not stable across releases:

ruff 0.15.22 ->  59 rules across  2 categories (E, F)
ruff 0.16.0  -> 413 rules across 38 categories

A routine pre-commit autoupdate crossing that boundary changes which rules run on code nobody has touched. That is exactly what fails the open autoupdate PR #173 here, on six findings:

Rule Count Location
RUF100 2 tests/test_resolve_config_source.py
I001 1 tests/test_resolve_config_source.py
SIM102 1 src/resolve_config_source.py
PIE810 1 src/resolve_config_source.py
PLW1510 1 src/resolve_config_source.py

Deliberately narrow

select = ["E", "W", "F", "C4", "UP"] is the widest set this repository already satisfies, so pinning changes no behaviour today.

It falls short of the organisation superset (E, W, F, I, B, C4, UP) by I and B. I checked both, and each is a code change rather than a configuration one:

  • B adds one B904 raise-without-from site at src/resolve_config_source.py:314, which needs a judgement about which exception context to preserve.

  • I flags the import block in tests/test_resolve_config_source.py, which deliberately calls sys.path.insert() before importing the module under test. The autofix is only a blank-line removal, but that file is mirrored byte-for-byte with harden-runner-block-action and carries an explicit banner:

    ╔══ SYNCHRONISED FILE — DO NOT EDIT IN ISOLATION ══╗
    Changes must land as paired PRs in both repositories together with the module.

    So it cannot be resolved from this repository alone, and I have deliberately not touched it.

Folding either into a configuration chore would obscure both.

A nice side effect of selecting E: the existing # noqa: E402 on line 30 becomes meaningful again. It is currently unused (hence the two RUF100 findings), because E402 is not enabled under ruff's default.

Note on category-level pinning

Ruff selectors are prefixes, so a future release adding new E* or F* rules will still enable them here. Pinning every rule code individually would be exhaustive and unmaintainable; the aim is to bound the blast radius — 38 categories back to five — not to freeze the set outright.

line-length and target-version are left unset on purpose. Both would change existing behaviour rather than pin it (target-version alters which pyupgrade rules fire).

Validation

  • ruff 0.16.0 check src testsAll checks passed!
  • ruff 0.15.22 check src testsAll checks passed! (so this is not a one-way door)
  • pre-commit run --all-files → every hook passes, including reuse lint on the new file
  • No code changes accompany this — configuration only

Unblocks #173.

Related

Paired with the identical change in harden-runner-block-action, which shares the same resolve_config_source module and test file.

This repository runs the ruff hook with no ruff configuration, so it
applies whatever ruff's default rule set happens to be.  That default
is not stable across releases: 59 rules across 2 categories in 0.15.22,
413 across 38 in 0.16.0.  A routine pre-commit autoupdate crossing that
boundary changes which rules run, on code nobody has touched.  That is
what currently fails the open autoupdate pull request here, on six
findings: RUF100, I001, PIE810, PLW1510 and SIM102.

The selection is deliberately narrow.  It is the widest set this
repository already satisfies, so pinning changes no behaviour today and
both ruff 0.15.22 and 0.16.0 report all checks passing.

It falls short of the organisation superset by "I" and "B", and both
gaps are code changes rather than configuration ones:

  - "B" adds one B904 raise-without-from site in
    src/resolve_config_source.py, needing a judgement about which
    exception context to preserve.
  - "I" flags the test module import block, which deliberately calls
    sys.path.insert() before importing the module under test.  That
    file is mirrored byte-for-byte with harden-runner-block-action and
    carries a "DO NOT EDIT IN ISOLATION" banner requiring paired pull
    requests, so it cannot be resolved from this repository alone.

Folding either into a configuration chore would obscure both.

The test per-file-ignores list rules that are not selected yet, so that
widening the selection later does not immediately flag every test
module.  PLR0917 is there because tests stacking @patch decorators
cannot satisfy it: unittest.mock injects each mock positionally, so a
keyword-only signature is impossible rather than merely inconvenient.

Co-authored-by: Claude <claude@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions requested review from a team and Copilot July 30, 2026 13:29
@github-actions github-actions Bot added the chore Code chores (dependency updates, etc) label Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces an explicit Ruff configuration to stabilize lint behavior across Ruff upgrades (notably the default-rule expansion between ruff 0.15.x and 0.16.x) and prevent unrelated pre-commit autoupdate runs from changing which lint rules apply.

Changes:

  • Adds a new .ruff.toml that pins lint rule selection to ["E", "W", "F", "C4", "UP"].
  • Keeps existing behavior stable by explicitly ignoring E501 and documenting the rationale for future widening via lint.per-file-ignores for tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions merged commit 2892883 into lfreleng-actions:main Jul 30, 2026
11 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

.ruff.toml:50

  • B008 is listed under [lint].ignore, but B rules are not enabled in select, so this ignore is currently redundant and can be confusing to maintainers. Since it has no effect today, consider removing it (it can be re-added when/if B is enabled).
ignore = [
  "E501", # line-too-long: the formatter owns line length
  "B008", # function call in argument default: typer/click idiom
]

.ruff.toml:62

  • There appears to be an extra blank line at the end of the file that contains whitespace. This can trip whitespace-cleanup hooks (e.g., trailing-whitespace) and cause pre-commit failures.
"tests/**/*.py" = ["S101", "ARG", "PLR0913", "PLR0917", "PLR2004"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Code chores (dependency updates, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants