fix: pin the store's images to the release tag they were reviewed at - #1
Merged
Merged
Conversation
`thumbnail` and `splash_image` pointed at `master`. Millennium's schema requires an absolute URL for both, so the submodule pin in the plugin database does not cover them: the store fetches whatever those URLs resolve to at the moment someone opens the listing. On a branch ref that means the image a reviewer approved and the image a user is shown are free to differ, indefinitely and without a trace in the submodule. Rather than hardcode a tag and create a release step to forget, `sync-version.ts` now rewrites the ref alongside the version it already writes. `@semantic-release/git` commits `plugin.json` during prepare and semantic-release tags that commit, so the tag these URLs name is the tag they are committed under, and `assets/` exists there. Every future release re-pins itself. `--check` gains the matching assertion, so a hand-edit back onto a branch ref fails CI instead of surfacing in the store listing. `$schema` is left alone deliberately — it tracks Millennium's `main` so editor hints follow the schema, and the test suite asserts that exclusion so widening the rule to "every raw URL" cannot freeze it by accident.
|
🎉 This PR is included in version 1.0.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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.
Closes the one open thread on PluginDatabase#213 — the review flagged that
thumbnailandsplash_imagepointed atmaster.Why the submodule pin doesn't already cover this
Millennium's schema requires an absolute URL for both fields, so they can't be repository-relative paths. The store fetches them when someone opens the listing, not when the plugin is reviewed. On a branch ref the image a reviewer approved and the image a user is shown are free to differ, indefinitely, without anything changing in the submodule the database pins.
Why this isn't a hardcoded tag
Hardcoding
v1.0.1would fightsemantic-release, which owns the version field, and would add a release step to forget. Insteadsync-version.tsrewrites the ref alongside the version it already writes, in the same prepare step.The chicken-and-egg it looks like it has isn't real:
@semantic-release/gitcommitsplugin.jsonduring prepare and semantic-release tags that commit, so the tag these URLs name is the tag they're committed under, andassets/exists there. Verified by simulating a release locally:Guards
--check, which CI already runs, gains the matching assertion. A hand-edit back onto a branch ref now fails the build rather than surfacing in the store listing:$schemais deliberately excluded — it tracks Millennium'smainso editor hints follow the schema as it changes. That exclusion is asserted in the tests, so widening the rule to "every raw URL in the manifest" can't freeze it by accident.repinAssetUrlthrows rather than passing a URL through unchanged. The schema names Imgur as an acceptable host, so an unparseable URL is plausible, and silently returning it would leave a mutable image in a released manifest while reporting success.Verification
tests/store-image-pins.test.tstsc --noEmitclean on bothtsconfig.jsonandscripts/tsconfig.jsonsync-version.ts --checkpasses against the committed treev1.0.0tag, so the pin resolves todayAfter merge
Merging cuts v1.0.1, which re-pins both URLs to
v1.0.1. The submodule in the PluginDatabase fork then needs bumping to that tag so the listing ships the pinned manifest.