Skip to content

DM-55493: scope PLR0917 for ruff 0.16 - #301

Merged
jonathansick merged 2 commits into
mainfrom
tickets/DM-55493-ruff-lint
Jul 27, 2026
Merged

DM-55493: scope PLR0917 for ruff 0.16#301
jonathansick merged 2 commits into
mainfrom
tickets/DM-55493-ruff-lint

Conversation

@jonathansick

Copy link
Copy Markdown
Member

Background

ruff 0.16.0 stabilized a batch of preview rules. Two of them now fire on this
repo's existing configuration without any code change on our side:

  • PLR0917 (too-many-positional-arguments) — flags callables taking more
    than five positional parameters.
  • ISC004 (explicit-string-concatenation) — no occurrences in ook.

Because ook selects broad rule families, these arrive automatically the moment
the pinned ruff version moves to 0.16. This PR gets ahead of that so the version
bump itself is a no-op.

The repo's pinned ruff-pre-commit rev is older than 0.16 and does not
surface these rules, so this was verified out-of-band with:

uvx ruff@0.16.0 check --select PLR0917,ISC004 .

The single finding

PLR0917 Too many positional arguments (7 > 5)
  --> src/ook/services/ingest/sdmschemas.py:69:15

SdmSchemasIngestService.create() — the async classmethod factory — accepted
seven positional parameters (http_client, logger, link_store,
sdm_schemas_store, gh_factory, github_owner, github_repo).

Disposition: fix, not ignore

Fixed by inserting a bare *, after cls, making every dependency
keyword-only. No parameter was removed, reordered, or renamed, and no noqa
was added.

A scoped ignore was rejected because the rule is pointing at something real
and the fix is free here:

  • All seven parameters are injected collaborators and config strings. None is
    "naturally positional" — there is no argument whose meaning is obvious from
    position alone, so nothing warrants staying positional. *, goes right after
    cls.
  • This restores internal consistency. The class's own __init__ is already
    fully keyword-only, and so is the sibling factory
    LsstTexmfIngestService.create() (src/ook/services/ingest/lssttexmf.py).
    SdmSchemasIngestService.create was the odd one out.
  • It removes a live footgun: logger and http_client were adjacent and
    trivially transposable at a call site, as were the two str parameters
    github_owner / github_repo.

Call sites

Grepped SdmSchemasIngestService across src/, tests/, and all other file
types, plus super().__init__( for subclass constructors.

There is exactly one invocation:

  • src/ook/factory.py:408Factory.create_sdm_schemas_ingest_service()

It already passed all seven arguments by keyword, so no call sites needed
changing
. The class has no subclasses, so there is no super().__init__() to
update. Remaining matches are prose only (a docstring in
tests/support/github.py, a test-data README, and a planning doc).

This is an internal service API — create() is not exported or part of ook's
HTTP surface — so the change is not breaking for any consumer.

Validation

  • uvx ruff@0.16.0 check --select PLR0917,ISC004 .All checks passed (was
    1 error).
  • uv run --only-group=nox nox -s typing test:
    • typing (mypy, 202 source files) → Success: no issues found
    • test451 passed, 1 skipped in 7:58, against the real Kafka and
      Postgres testcontainers.
  • Pre-commit ruff check / ruff format hooks passed on commit.

A signature change that broke the caller would have failed mypy and the ingest
tests; both are green.

Notes

  • No changelog fragment. The repo uses scriv, but this changes only an
    internal service constructor signature — no behavior, HTTP API, config, or
    dependency change is user-visible.
  • Out of scope: ruff 0.16.0 also reports one pre-existing RUF036
    (None not at end of union) at src/ook/cli.py:63, which is present on
    main and untouched here.

ruff 0.16.0 stabilizes PLR0917 (too-many-positional-arguments), which
flags the create() classmethod factory at 7 positional parameters.

Insert a bare `*,` after `cls` so every dependency is passed by
keyword. This matches the sibling LsstTexmfIngestService.create and the
class's own __init__, both of which are already keyword-only. The single
call site (Factory.create_sdm_schemas_ingest_service) already used
keyword arguments, so no caller changes were required.
ruff-shared.toml already sets [lint.isort] split-on-trailing-comma = false,
so the pyproject copy is dead config. Clears the toml-not-redundant check.
@jonathansick
jonathansick enabled auto-merge July 27, 2026 20:28
@jonathansick
jonathansick merged commit da94db6 into main Jul 27, 2026
4 checks passed
@jonathansick
jonathansick deleted the tickets/DM-55493-ruff-lint branch July 27, 2026 20:35
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