Skip to content

feat: restore pylint to the quality checks - #294

Open
farhan wants to merge 6 commits into
mainfrom
farhan/restore-pylint-289
Open

feat: restore pylint to the quality checks#294
farhan wants to merge 6 commits into
mainfrom
farhan/restore-pylint-289

Conversation

@farhan

@farhan farhan commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Important

Generated with Claude, then manually reviewed (both this PR description and the code commits).

Related

Summary

Restores pylint to the quality gate alongside ruff. Per #289, ruff is not a drop-in replacement for pylint — it does not run the edx_lint plugins and does not catch the class of issues (undefined attributes, type mismatches, etc.) that pylint does. Ruff stays responsible for style, imports, and formatting; pylint is added back for the deeper static analysis.

Ruff rules disabled in this PR

To avoid pylint/ruff double-ownership and a direct conflict, two ruff rules are disabled so pylint is the single authority for them:

  • F401 (unused-import)newly disabled here. Reason: ruff's F401 wants __init__.py re-exports marked with a redundant import X as X alias, but pylint rejects that exact alias as C0414 (useless-import-alias) — a direct, unavoidable conflict. Disabling F401 lets the __init__ files use plain imports; pylint's W0611 covers unused-import instead.
  • E501 (line-too-long)was already disabled before this PR (previously commented "handled by formatter"). Its comment is updated to reflect that pylint's C0301 now owns line-length enforcement.

Everything else in ruff's select (E, W, F minus F401, I, B, C4, UP, DJ) stays active. This mirrors how other openedx repos that run both tools (openedx-events, openedx-filters) divide the work — pylint owns unused-import and line-length, ruff owns style.

Changes, commit by commit

  • feat: integrate pylint into the quality checks

    • Add edx-lint to the quality dependency group (transitively pulls pylint, pylint-django, pylint-celery, pylint-plugin-utils).
    • Add pylintrc + pylintrc_tweaks (regenerated with current edx-lint; load-plugins = edx_lint.pylint, ignore = migrations).
    • Run pylint xblocks_contrib xblock_pdf in the quality tox env.
    • Regenerate uv.lock (additive only).
  • refactor: rename Makefile linttarget toquality``

    • Rename the lint target to quality to match the tox env it runs (tox -e quality). No external callers.
  • chore: let pylint own line-length and unused-import in ruff

    • Add F401 and E501 to ruff's ignore list with a short rationale comment (see section above).
  • refactor: use plain imports in package init files

    • Drop the redundant import X as X re-export aliases now that ruff no longer enforces F401; use plain imports. Clears pylint C0414 and restores the pre-modernization style.
  • refactor: remove stale pylint useless-suppression comments

    • Remove # pylint: disable= guards pylint now reports as useless-suppression (I0021) — raise-missing-from guards on raise ... from None, and too-many-arguments / too-many-positional-arguments guards on signatures now within limits.
    • Two special cases preserved: problem/tests/__init__.py keeps its still-needed arguments-differ / unused-argument guards (only the stale part dropped); lti/tests/helpers.py genuinely exceeds the positional-arg limit, so its guard is moved onto the def line where it actually applies.
  • fix: wrap over-length line flagged by pylint line-too-long

    • Extract a slice into a local so an error-message f-string fits the 120-char limit (C0301).

Testing

tox -e quality passes end to end:

ruff check .            → All checks passed!
ruff format --check .
pylint xblocks_contrib xblock_pdf
make selfcheck          → The Makefile is well-formed.
quality: OK

CI already runs uv run tox with quality in the matrix, so pylint runs there automatically — no workflow change needed.

@farhan
farhan marked this pull request as draft August 21, 2026 08:11
Restore pylint alongside ruff in the quality gate. Per #289,
ruff is not a drop-in replacement for pylint: it doesn't run the edx_lint plugins
or catch the class of issues (undefined attributes, type mismatches, etc.) pylint
does. Ruff continues to handle style and import ordering.

This change is integration/config only -- no source code changes:
- Add `edx-lint` to the `quality` dependency group (pulls pylint plus the
  pylint-django / pylint-celery / pylint-plugin-utils plugins).
- Add `pylintrc` and `pylintrc_tweaks` (regenerated with the current edx-lint;
  `load-plugins = edx_lint.pylint`, `ignore = migrations`).
- Run `pylint xblocks_contrib xblock_pdf` in the `quality` tox env.
- Regenerate `uv.lock` for the new dependencies (additive only).

CI already runs `uv run tox` with `quality` in the matrix, so pylint runs there
automatically. Re-enabling it surfaces pre-existing findings in the source, which
will be addressed separately.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@farhan
farhan force-pushed the farhan/restore-pylint-289 branch from 5efa33b to 23291e6 Compare August 21, 2026 10:47
farhan and others added 5 commits August 21, 2026 15:53
Align the Makefile target name with the tox env it runs (`tox -e quality`) and
with the quality gate terminology. The target had no external callers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add E501 and F401 to ruff's ignore list so pylint (C0301 / W0611) is the single
authority for line-length and unused-import. This resolves the ruff/pylint
conflict on __init__ re-exports (ruff F401 wants a redundant `import X as X`
alias, which pylint rejects as C0414) and keeps line-length enforcement in one
place.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that ruff no longer enforces F401, drop the redundant `import X as X`
re-export aliases and use plain imports (merging same-module names onto one
line). This matches the pre-modernization style and clears pylint's C0414
(useless-import-alias) on these files.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the `# pylint: disable=` guards that pylint now reports as
useless-suppression (I0021) -- warnings that no longer fire under the current
pylint, so the suppressions do nothing:

- raise-missing-from guards on `raise ... from None` statements
- too-many-arguments / too-many-positional-arguments guards on signatures now
  within the configured limits

Two special cases preserved:
- problem/tests/__init__.py keeps its still-needed `arguments-differ` and
  `unused-argument` guards; only the stale too-many-positional-arguments part
  is dropped.
- lti/tests/helpers.py's TestToolConsumer.__init__ genuinely exceeds the
  positional-argument limit, so its guard is moved onto the `def` line (where it
  actually applies) instead of being removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extract the context slice into a local so the error message f-string fits within
the 120-char limit. pylint (C0301) now owns line-length since ruff cedes E501.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@farhan farhan self-assigned this Aug 21, 2026
@farhan
farhan marked this pull request as ready for review August 21, 2026 13:10
@farhan farhan moved this to 👀 In review in Aximprovements Team Aug 21, 2026
@farhan
farhan requested review from a team and kdmccormick August 21, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

Restore pylint: ruff is not a drop-in replacement

1 participant