Repository navigation
TWE-728: ruff.toml: extend-select over select, resolve RUF012 noise - #472
Open
nickmoreton wants to merge 21 commits into
Open
nickmoreton wants to merge 21 commits into
nickmoreton wants to merge 21 commits into
Conversation
Bumps the Wagtail pin from ~7.4 to ~8.0 (major) via `poetry add`, refreshing the lockfile for Wagtail and its transitive dependencies. Folds in the specifier-normalisation pass (Poetry `^` carets rewritten to `~` tilde pins at each package's installed major.minor). wagtail-upgrade-audit 0.19.2 has no `wagtail80` ruleset yet (its known audits top out at `wagtail74`), so no automated findings were available for this major version. `manage.py check` and the test suite were run as validation instead and are green. Django stays at ~5.2 (patch-only bump, handled in the next commit). Co-authored-by: Support Skills <support@torchbox.com>
Advances every eligible specifier floor to the latest supply-chain- cool-down-allowed minor and refreshes poetry.lock accordingly (Django and Wagtail excluded — they move only through the audit gate, handled in the previous commit). Notable floor bumps: django-redis ~6.0 → ~7.0 (major), django-debug-toolbar ~6.3 → ~7.1 (major), plus minor bumps to gunicorn, django-pattern-library, django-permissions-policy, django-phonenumber-field, wagtailmedia, sentry-sdk, beautifulsoup4, django-upgrade, ruff, pymdown-extensions, faker, and wagtail-factories. `manage.py check` and the full test suite (141 tests) pass with no new warnings. Co-authored-by: Support Skills <support@torchbox.com>
- Bump devDependencies/dependencies to latest via ncu, held to the ESLint ceiling (eslint, eslint-webpack-plugin) and the npm cool-down clamp (jest, jest-environment-jsdom, sass-loader, webpack). - Hold typescript at ^6.0.3 (<7): ts-jest's latest release (29.4.12) still peer-depends on typescript <7, so a TS7 install fails ERESOLVE. Logged in docs/upgrading.md with the lift condition. - Drop dead IE11 scaffolding (presumptive drop, confirmed green): browserslist ie11 entries, the foreach/closest polyfills, the svgo ie11 closing-tags override, and raise the TS target from es5 to es2016. - Fix fallout from the bump: reformat .github/pull_request_template.md (prettier 3.9.6), drop an eslint-disable in dynamic-hero.js left over from an older swiper resolution issue (swiper 12 -> 14). Co-authored-by: Support Skills <support@torchbox.com>
Co-authored-by: Support Skills <support@torchbox.com>
Co-authored-by: Support Skills <support@torchbox.com>
…e narrative Co-authored-by: Support Skills <support@torchbox.com>
Co-authored-by: Support Skills <support@torchbox.com>
Co-authored-by: Support Skills <support@torchbox.com>
ruff 0.16 expanded its default rule set from just E/F to nearly every category. ruff.toml used extend-select, so the 0.16 default drift leaked in underneath the explicit list, surfacing 459 new findings (dominated by RUF012 on idiomatic Wagtail/Django patterns) that were never actually selected. Switching extend-select to select closes the config over ruff's defaults, so it no longer drifts with them. Confirmed 0 findings under ruff 0.16.5 with the fix; pre-commit hook bumped to v0.16.5 to match the poetry-managed version, lifting the hold from 96fd005. Co-authored-by: Support Skills <support@torchbox.com>
Move the colour, breakpoint and spacing theme, content sources and the forced-colors variant into tbx/static_src/css/tailwind.css. Split the webpack style rule so plain CSS skips sass-loader (Dart Sass cannot parse Tailwind's --color-*: initial reset). Remove the JS config from the Docker build and compose mount, and the ESLint ignore that only existed for it. Rewrite the upgrading.md Node hold entries as state and remove the Tailwind section, whose content now lives in the CSS file and existing checklists. Compiled output verified equivalent by diffing the full webpack build before and after. Co-authored-by: Support Skills <support@torchbox.com>
Co-authored-by: Support Skills <support@torchbox.com>
… (2.8.0) Co-authored-by: Support Skills <support@torchbox.com>
- wagtail-markdown from 0.14.1 to 0.15.0 - wagtailmedia from 0.18.1 to 0.19.1 - django-debug-toolbar from 7.1.1 to 8.0.0
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…eview Testing the suggested follow-up from the unresolved review thread on ruff.toml line 4: go back to extend-select (Ruff's defaults + our list) instead of select (only our list), to see what Ruff 0.16.5's expanded defaults actually surface. Not yet fixed up - this is to inspect the effects before deciding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ruff.toml was left at py313 after the Python 3.13 -> 3.14 bump, so pyupgrade rules weren't applying 3.14-era syntax. Bumping it surfaced 10 auto-fixable findings (UP037/UP045), fixed with ruff --fix and ruff format: unquoting forward-reference type hints, switching Optional[X] to X | None, and (per PEP 758, new in 3.14) dropping the parens on multi-exception except clauses. tbx/sitemap/models.py would also reformat its except clause, but with extend-select active on this branch that drags two pre-existing RUF012 findings on unrelated lines into the pre-commit hook, so it's left untouched here pending the RUF012 decision. Doesn't change the RUF012-dominated 506-finding baseline from the extend-select experiment on this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per zerolab's PR #456 review: keep extend-select, ignore RUF012 (mutable class attributes are idiomatic in Django migrations and Wagtail page/model config, not real bugs), then fix everything else Ruff's expanded 0.16 defaults surfaced: - fabfile.py: explicit check=False on four subprocess.run calls, readlines() loop -> direct file iteration, local-time filename timestamp exempted via per-file-ignore (DTZ005) - tbx/core/blocks.py, update_youtube_embeds.py: drop the redundant exception object from logger.exception calls (TRY401) - tbx/core/templatetags: join() -> f-string/literal (FLY002), unnecessary int(round(...)) (RUF046) - tbx/core/views.py: static robots.txt join -> literal string - tbx/impact_reports/*: dropped an accidental +x bit on four plain Python modules (EXE002) instead of adding a shebang - tbx/navigation/signals.py: narrowed a blind except Exception to the two exceptions Page.get_site() can actually raise, and dropped a now-redundant local Site import - tbx/people/factories.py: TypeError instead of ValueError for an isinstance check (TRY004) - tbx/people/models.py: prefix unused get_or_create() tuple member with _ (RUF059) - tbx/core/tests/test_util_tags.py: per-file-ignore for DTZ001 - every call immediately chains .date(), so tzinfo is discarded regardless ruff check is clean. Full test suite (232 tests) passes under docker compose with the real Postgres DB. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zerolab
reviewed
Oct 13, 2026
zerolab
reviewed
Oct 13, 2026
- Handle the ClassVar typing
Member
|
@nickmoreton the tests are failing on a migration check. Other than that LGTM (I have nitpicks, but will submit a follow up PR once this is all done and dusted) |
Collaborator
Author
I preferred to not have a new migration: 8916ba8 |
zerolab
requested changes
Oct 14, 2026
| "tbx/core/utils/scripts/run_flightpath_status_check.py" = ["T201"] | ||
| "tbx/core/tests/test_util_tags.py" = ["DTZ001"] | ||
| "*/migrations/*.py" = ["RUF012"] # Django migration dependencies/operations lists are idiomatic here, not bugs | ||
| "tbx/taxonomy/models.py" = ["RUF012"] # Meta.ordering as a tuple would need a migration for no behaviour change |
Member
There was a problem hiding this comment.
I would rather this was in the taxonomy Meta
ordering: ClassVar[list[str]] = ["sort_order"]without the need for full per-file ignore. Or at least do # noqa: RUF012 inline in the model file
nickmoreton
force-pushed
the
support/TWE-728-wagtail-80-upgrade
branch
from
October 21, 2026 08:21
c5a0420 to
0aac0a3
Compare
This branch has not been deployed
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.
Summary
Follow-up from #456's unresolved review thread, where zerolab suggested going back to
extend-select(Ruff's defaults + our additional rules) instead ofselect(only our list), so we don't lose new default rules Ruff adds in future releases.ruff.tomlback toextend-select, which re-surfaced Ruff 0.16.5's expanded default rule set (506 findings).RUF012toextend-ignore— it accounted for 472 of those findings, almost all on idiomatic Django migration/Wagtail page-config patterns (mutable class attributes that are never mutated), not real bugs.check=Falseonsubprocess.runcalls, redundant exception objects dropped fromlogger.exception,str.join→ f-string/literal, an accidental+xbit removed from fourimpact_reportsmodules, a blindexcept Exceptionnarrowed to the exceptionsPage.get_site()can actually raise, and a few smaller type/naming fixes.target-versiontopy314to match the platform upgrade in the base branch, which surfaced 10 auto-fixablepyupgradefindings (unquoted forward-ref type hints,Optional[X]→X | None, and PEP 758's unparenthesized multi-exceptionexceptclauses).Test plan
ruff check .is cleandocker composeagainst a real Postgres DB🤖 Generated with Claude Code