build: prepare for Renovate and add a dependency cooling-off period - #67
Open
feanil wants to merge 9 commits into
Open
build: prepare for Renovate and add a dependency cooling-off period#67feanil wants to merge 9 commits into
feanil wants to merge 9 commits into
Conversation
`apps.py` imports `edx_django_utils` and `pipeline.py` imports `crum`, but neither was in `[project].dependencies`. They resolved anyway because the `test-base` dependency group pulls in `edx-django-utils`, which brings `django-crum` transitively -- so the omission was invisible in development and in CI, and would only surface as an ImportError for someone installing the published wheel into an environment without the test groups. The uv.lock change is metadata only: both packages were already resolved, so only this project's own declared dependency list gains the entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renovate skips any PEP 621 dependency that has no version specifier at all
(`skipReason: "unspecified-version"`), dropping it before it ever queries
PyPI -- a correct `lockedVersion` in uv.lock does not help, because the
manifest is what Renovate reads as the reference. Since almost every entry
here was a bare name, Renovate would have produced no update PRs for them.
`>=0` is the PEP 440 equivalent of the "*" ranges already used in
frontend-plugin-sample/package.json; PEP 440 has no bare wildcard, as
`Django==*` is a parse error. It keeps these deliberately unconstrained --
the Open edX platform pins them itself, and a plugin that narrows them
causes resolution conflicts when installed into edx-platform -- while
stating that intent explicitly instead of leaving it implied by omission.
Real floors were considered and rejected: derived from uv.lock they would
be guesses, since the lock records the newest resolvable version rather
than the oldest supported one. `Django>=5.2` would block installation on a
platform running Django 4.2, quite possibly wrongly.
The one genuine floor is `edx-lint>=6.0`, which is verifiable rather than
guessed: `edx_lint write_uv_constraints` does not exist before 6.0. The
`test` and `django60` Django pins are untouched -- they define the test
matrix.
Every uv.lock change is a metadata-only rewrite of a `{ name = "x" }` entry
to `{ name = "x", specifier = ">=0" }`. No resolution moved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wait seven days before resolving to any newly published release. A hijacked release is usually yanked within a few days, so for a repo where nothing is urgent this is cheap insurance against installing a compromised version in the window before anyone notices. Set in the package managers rather than in Renovate, because Renovate cannot enforce a cooldown on a lockfile refresh: it delegates that to uv or npm and never enumerates the individual releases, so `minimumReleaseAge` does not apply to it. Configuring the resolvers instead covers transitive dependencies as well as direct ones, and means `make upgrade` or `npm install` on a laptop behaves exactly like the bot. Two .npmrc files rather than one at the repo root: npm reads the project config only from the directory holding package.json and does not walk up. Verified: `uv lock` is idempotent afterwards, `uv lock --check` and `uv sync --locked` still pass, and `npm ci` plus `npm run build` are unaffected in both npm packages. uv records the window in uv.lock as `[options] exclude-newer-span` and re-evaluates it per resolve; a widening window cannot invalidate the lock, since exclude-newer only restricts which candidates are eligible and never forces an upgrade. Two caveats recorded in the comments. `min-release-age` needs npm >= 11.10.0, so it does nothing in brand-sample until its .nvmrc moves off Node 20 -- Node 24 bundles npm 11.19.0. And Renovate's own `minimumReleaseAge` must be kept at the same value, or it will offer versions the resolvers refuse and leave empty branches behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
=======================================
Coverage 93.04% 93.04%
=======================================
Files 16 16
Lines 503 503
Branches 18 18
=======================================
Hits 468 468
Misses 24 24
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
feanil
marked this pull request as ready for review
September 9, 2026 13:37
feanil
force-pushed
the
feanil/renovate-prework
branch
from
September 9, 2026 14:03
6b93210 to
f85520e
Compare
feanil
enabled auto-merge (rebase)
September 9, 2026 14:04
feanil
disabled auto-merge
September 9, 2026 14:04
brand-sample had no CI at all, which matters more than it looks. Its
compiled dist/light.min.css is committed, and brand-sample/README.md tells
consumers to load it directly out of this repository:
brandOverride: 'https://cdn.jsdelivr.net/gh/openedx/sample-plugin@main/brand-sample/dist/light.min.css'
That URL is pinned to @main, so whatever lands on the default branch is
what MFEs following the brand example immediately start loading. dist/ is
therefore a published artifact rather than a build leftover.
Nothing rebuilt or checked it. Updating package-lock.json does not
regenerate dist/, so a dependency bump that changed the build output would
leave the CSS people load out of step with the tokens in this repo, with
nothing to catch the drift.
The job installs from the lockfile, rebuilds, and diffs dist/ against the
build output. When it fails, the fix is to run `npm run build` in
brand-sample and commit the result.
Also moves .nvmrc from Node 20 to 24. Node 20 is end-of-life, and it bundles
npm 10.8.2, which silently ignores the `min-release-age` cooling-off setting
added in the previous commit -- so brand-sample got no benefit from it until
now. Node 24 bundles npm 11.19.0. Verified that dist/ still builds
byte-identically under Node 24, so this needs no accompanying rebuild.
The workflow is workflow_call only, because the next commit adds a ci.yml
that aggregates every check into one required status check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Branch protection matches required status checks by name, which made the required list both fragile and incomplete. Fragile, because naming the backend jobs individually means spelling out every matrix leg -- `tests (ubuntu-latest, 3.12, django52)` and four more -- so adding or retiring a Python or Django version quietly breaks protection until someone remembers to update the repository settings. Incomplete, because the frontend and tutor jobs were both called `build`. Two check runs with one name cannot be told apart in a required list, so neither was ever required. Only the five backend contexts and openedx/cla were, which means a pull request touching nothing but frontend files could merge on the strength of checks that could not have been affected by it. That gap matters much more once dependency updates start merging themselves, because GitHub's auto-merge waits for required checks and nothing else. So ci.yml now runs on pull requests, invokes each per-package workflow, and ends in a job called "all required checks passed" that succeeds only if all of them did. That is the only name branch protection has to know, and it never changes whatever happens to the matrix. The per-package workflows become workflow_call only, so they run once per pull request rather than twice. release.yml calls ci.yml instead of the three workflows separately, so a release runs exactly the checks that were required to merge, and picks up brand-ci -- and anything added later -- without a change there. The aggregating job runs `if: always()`, because a skipped required check never reports at all and would block the pull request forever rather than failing it. The aggregation is five lines of shell rather than a third-party action. This job is the one thing standing between a pull request and main, so it is the last place worth adding a dependency -- particularly in a change whose purpose is to reduce how much unreviewed third-party code we pull in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`make upgrade` calls `edx_lint write_uv_constraints`, and that subcommand does not exist before edx-lint 6.2.0 -- so with edx-lint pinned at 6.0.0 the target has been failing, printing the usage text instead of regenerating anything. That is also why the previous commit's `edx-lint>=6.0` floor was wrong; the real floor is 6.2. Raising it surfaced two files edx-lint owns that had drifted: [tool.uv].constraint-dependencies was missing three of edx-lint's global constraints -- social-auth-app-django<6.0.0, social-auth-core<5.0.0 and pip<26.2.1. Nothing had regenerated it since those were added upstream, which is precisely the failure the next commit puts a check around. pylintrc was still stamped "Generated by edx-lint version: 5.6.0", predating the pii_annotation_check plugin's requirement that `pii_terms` be configured. Under 6.2.0 pylint aborted with "The 'pii_terms' setting must be configured", taking models.py down with an astroid-error. Regenerated with `edx_lint write pylintrc`, which adds the [PII] section and enables pii-invalid-no-pii-annotation. The file says DO NOT EDIT, so this is entirely tool output plus the local pylintrc_tweaks. `tox -e quality`, `-e docs` and `-e pii_check` all pass afterwards. The two test envs segfault locally, but they do so identically on an unmodified checkout of main, so that is a local environment problem rather than anything from this change -- CI on main is green, and `pytest` run directly passes all 23 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
[tool.uv].constraint-dependencies is generated by edx-lint from its global common_constraints.txt plus this repo's [tool.edx_lint].uv_constraints, and nothing regenerated it automatically -- the previous commit found it three constraints behind upstream. The scheduled workflow that used to run `make upgrade` is being removed, and its replacement cannot take over this job: the Mend-hosted Renovate app has no postUpgradeTasks, so it cannot run `edx_lint write_uv_constraints` after an update. Reinstating a cron that opens a pull request is not much better, because a workflow using the default GITHUB_TOKEN produces pull requests that do not trigger CI. So instead of regenerating on a schedule, fail when it drifts. The loop closes itself: edx-lint is a managed dependency, so the pull request that bumps it is the one that goes red if edx-lint's global constraints moved along with it. The drift is reported by the change that caused it, and automerge is blocked until someone regenerates. The target diffs against a scratch copy rather than checking `git diff`, so it reports only this drift and not whatever else is uncommitted in the working tree, and it restores the file when it fails rather than leaving the regenerated version behind. Also fixes `make upgrade` to ask for 'edx-lint>=6.2' rather than a bare `edx-lint`, so it cannot quietly resolve to a version without the subcommand it depends on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The requirements/ directory went away when this repo moved to uv.lock, but several references to it survived: - tox.ini told pytest not to recurse into `requirements` - .gitignore still ignored requirements/private.in and .private.txt - CLAUDE.md described the dependency commands without mentioning uv at all - pyproject.toml pointed twice at docs/how-tos/adding-a-matrix-dependency.rst, which does not exist -- docs/how-tos/index.rst links to the real thing on docs.openedx.org, so these now point there directly Also records in CLAUDE.md that Renovate owns routine bumps now, and that the 7-day minimum release age is configured in three places that have to stay in agreement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.readthedocs.yaml still asked for `requirements/doc.txt`, which stopped existing when the repo moved to uv.lock, so the config could not have worked. Switched to Read the Docs' native uv support -- `method: uv` with `command: sync` and the `doc` dependency group -- which resolves from the committed uv.lock, so hosted docs would build against the same pinned versions as CI and inherit the minimum release age recorded there. Worth knowing: this file is dormant, not broken in production. Read the Docs reads .readthedocs.yaml from the repository root and this one is a directory down, so nothing reads it unless a project is created and pointed at this path. No project exists -- both plausible subdomains 404 and nothing in the repo references one -- and the docs are built with fail_on_warning on every pull request by `tox -e docs` regardless. A note in the file now says so, along with the fact that its paths are relative to backend-plugin-sample/ rather than the repository root. Kept rather than deleted so that turning on hosted docs later is a matter of creating the project instead of rewriting the config. Untested by definition: with no project to build it, Read the Docs would be the first thing to actually run this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feanil
force-pushed
the
feanil/renovate-prework
branch
from
September 9, 2026 19:19
f85520e to
dfa8771
Compare
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
1 participant
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.
Groundwork for moving dependency management to Renovate. The Renovate config
itself is a small follow-up PR.
The main change is a 7-day minimum release age on every dependency, so a
freshly published release can't be pulled in immediately. It's set in the
package managers rather than in Renovate —
exclude-newerinpyproject.tomlandmin-release-agein two new.npmrcfiles — so itcovers transitive dependencies too, and local
make upgradebehaves the sameway as the bot will. Renovate's own setting has to be kept at the same value.
Alongside that:
>=0, since Renovate skips any dependency withno version specifier at all. Same intent as the
"*"ranges on the npm side.all required checks passed, replaces thefive per-matrix-leg contexts. The frontend and tutor jobs were both named
build, so neither could ever be required.brand-samplegets CI. Its compileddist/light.min.cssis committed, andthe README tells consumers to load it straight from this repo via jsdelivr,
pinned to
@main. Nothing rebuilt or checked it, so a dependency bump thatchanged the build output would leave the CSS people load out of step with the
tokens here. The new job rebuilds and diffs
dist/.edx-lintto 6.2, whichmake upgradeneeds and never had, plus amake check-constraintsguard so the files it generates can't drift again.requirements/references,and a dormant
.readthedocs.yamlpointing at a deleted file.The commit messages have the reasoning for each.
Merging
Required checks on
mainare already updated toall required checks passed,so this can merge as-is. Merging cuts a patch release.