ci: gate on ruff's default rules, and clear the nine findings - #145
Merged
Conversation
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
Issue #77 asked for a repo-wide ruff gate once the repository was clean enough to have one. It is, at ruff's default rule set, so this is that gate. Nine F401s were sitting here. Eight were introduced the same day, by the WCM integrations: imports that became unused when those modules switched to the SDK's artifact_digest, and typing leftovers from threading annotations through. They were tested and never linted, because nothing lints this repository. That is the gap a gate closes and a code review does not. The default set (E4, E7, E9, F) is the deliberate scope. It catches defects rather than preferences: undefined names, unused imports, f-strings with no placeholders, duplicate arguments. Wider selections are style, and several integrations here are contributed by their vendors under CONTRIBUTING rule 5, "You maintain it". Failing somebody's pull request on import sorting is how a self-serve submission path stops being self-serve. A wider set, if wanted, should arrive with its own fix pass rather than by being switched on for the next contributor to discover. Worth recording for anyone who reruns #77's numbers: they will not reproduce. A bare `ruff check` on this machine reports 128 findings in the three directories the issue names, because ruff resolves configuration from outside the repository and this one carries none. Against the default set the same directories are clean. The gate pins ruff 0.16.3 and states its selection explicitly so the number means the same thing on every machine. Paths are listed rather than ".", so a vendored or generated tree added later does not silently join the gate. 401 tests still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014NL8o3PXq6kfs2SdmBv6ak
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the last actionable item in #77.
The nine findings, and where they came from
Eight of nine were introduced the same day, by the WCM integrations: imports that became unused when those modules switched to the SDK's
artifact_digest, and typing leftovers from threading annotations through.They were tested and never linted, because nothing lints this repository. That is the gap a gate closes and a code review does not.
Scope: default rules only, on purpose
E4, E7, E9, Fcatches defects rather than preferences: undefined names, unused imports, f-strings with no placeholders, duplicate arguments.Wider selections (
UP,I,ANN,D,FA) are style, and several integrations here are contributed by their vendors under CONTRIBUTING rule 5, "You maintain it". Failing somebody's PR on import sorting is how a self-serve submission path stops being self-serve. A wider set, if wanted, should arrive with its own fix pass rather than being switched on for the next contributor to discover.#77's numbers will not reproduce, and that is worth knowing
A bare
ruff checkon my machine reports 128 findings in the three directories the issue names, not 21. The reason is that ruff resolves configuration from outside the repository when the repository carries none, so the effective rule set depends on whose machine runs it. Against the default set those same directories are clean.The gate pins
ruff==0.16.3and states its selection explicitly, so the number means the same thing everywhere.Paths are listed rather than
., so a vendored or generated tree added later does not silently join the gate..nox/is already excluded by ruff's defaults, which matters here because it holds installed site-packages.Rest of #77
Verified separately and reported on the issue: the stack-trace exposures were fixed in #132, the ruff errors in
sentinel/comply54in #137, and every GitHub action and thesentinelDockerfile base image are digest-pinned. Unpinnedpip installremains and is arguably deliberate, since the conformance workflows exist to test against current releases.401 tests pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_014NL8o3PXq6kfs2SdmBv6ak