style: clear ai-failure-notifier's findings under the team's rule set - #7
Open
tonyandrewmeyer wants to merge 2 commits into
Open
tonyandrewmeyer wants to merge 2 commits into
tonyandrewmeyer wants to merge 2 commits into
Conversation
`style/python.md` names seventeen ruff rule groups; the monorepo root turns on eight. Turning on the rest finds eight things here, which is what has been keeping the standard out of the root config. None of them is a behaviour change. * Two docstring summaries in the imperative mood (`D401`). * `fetch_issue_texts` builds its list with `extend` rather than a loop that appends (`PERF401`). * `resolve_origin` discards `find_run_markers`' third value deliberately -- the passed-in issue wins -- so it is `_` rather than a name nothing reads (`RUF059`). * One nested `with` in a test becomes one `with` and two contexts (`SIM117`). * The two `subprocess` findings are marked rather than changed: `gh` really is called by name and resolved from PATH, and the argv really is a list this module builds, so `S603`/`S607` are answered with a `noqa` and the reason next to it, the same way `S607` already was. The test conftest imports `subprocess` only to patch it out, which is what its `S404` says.
tonyandrewmeyer
marked this pull request as ready for review
September 14, 2026 23:54
`# ruff: ignore[start-process-with-partial-path]` says what is being allowed; `# noqa: S607` needs a lookup. Ruff reads the name as well as the code, matches on it (naming a different rule does not suppress), and `RUF100` reports an unnecessary one as an unused suppression, so nothing is lost by the change. Also converts `_openrouter`'s pre-existing `S310`, which this branch did not otherwise touch: one package with two suppression syntaxes is worse than either. One thing to know: the directive is preview-gated. Under `preview = false` it is ignored and the findings come back. The root config sets `preview = true` and its comment already calls that load-bearing, so this is a second thing depending on it. Co-authored-by: James Garner <james.garner@canonical.com>
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
2 participants
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.
style/python.mdnames seventeen ruff rule groups as the set the team standardised on, and the monorepo root turns on eight of them. Turning the rest on finds eight things inai-failure-notifier, and those eight are the only reason the standard is not at the root already. None of them is a behaviour change.D401).fetch_issue_textsbuilds its list withextendrather than a loop that appends (PERF401).resolve_origindiscardsfind_run_markers' third value on purpose - the passed-in issue wins, because a marker we failed to find does not make the issue the wrong one - so it is_rather than a name nothing reads (RUF059).within a test becomes onewithwith two contexts (SIM117).subprocessfindings are marked rather than changed.ghreally is called by name and resolved from the runner's PATH, and the argv really is a list this module builds with no shell anywhere, soS603andS607get anoqaand the reason beside it - the same treatmentS607already had. The test conftest importssubprocessonly to patch it out, which is what itsS404is about.68 tests pass. The rule set itself moves to the root in #6, which is red on lint until this lands.