mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-04 18:28:06 +00:00
docs(sports): address review — test matrix, and compatible_versions
Both CodeRabbit findings on #427 hold up against the code; one sub-point was already moot. 1. The compatibility regression test was described as one case (bundled copy removed on an old core) when it needs four. The case that actually matters is the one that was missing: an adopted plugin loading *with* its bundled copy on an old core, which is the entire basis for claiming B5 is safe to run ahead of the gate. Now a 2x2 table. The assertion was also wrong. "Fails loudly and specifically" is aspirational -- PluginManager.load_plugin catches ModuleNotFoundError, so nothing propagates and it fails into PluginState.ERROR with one log line. A test expecting a raise would pass for the wrong reason. Specified as PluginState.ERROR plus the exact missing module path, which is also what the rest of this document already says the failure looks like. 2. `compatible_versions` -- not `ledmatrix_min_version` -- is the canonical contract: schema/manifest_schema.json requires it, all 42 published manifests carry it, and it holds semver ranges ([">=2.0.0"] in 41, [">=1.0.0"] in 7-segment-clock). The gate as merged reads only the floor. Harmless today: no manifest uses an upper bound, and the two fields agree everywhere except 7-segment-clock. But the schema's range syntax permits upper bounds, so a plugin declaring ["2.0.0 - 2.9.9"] would be installed on 3.2.0 regardless. Recorded as a named gap the gate must close before B6, and the migration step now has to reconcile both fields across every manifest the gate can refuse. Skipped, with reason: the finding also asked to migrate the deprecated top-level `ledmatrix_version`. No manifest carries it -- verified across all 42 -- so there is nothing to migrate. Documentation only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
This commit is contained in:
co-authored by
Claude Opus 5
parent
62c9cbb184
commit
9972111659
@@ -239,6 +239,29 @@ agree, and keep them agreeing; reconsider the `< 2.0.0` skip; migrate manifests
|
|||||||
from `ledmatrix_min` to `ledmatrix_min_version`; and add the install/update
|
from `ledmatrix_min` to `ledmatrix_min_version`; and add the install/update
|
||||||
compatibility gate that B6 depends on.
|
compatibility gate that B6 depends on.
|
||||||
|
|
||||||
|
#### Two fields express compatibility, and the gate only reads one
|
||||||
|
|
||||||
|
`compatible_versions` is the canonical contract: `schema/manifest_schema.json`
|
||||||
|
**requires** it, all 42 published manifests carry it, and it holds semver
|
||||||
|
*ranges* — `[">=2.0.0"]` in 41 of them, `[">=1.0.0"]` in `7-segment-clock`.
|
||||||
|
`ledmatrix_min_version` is the optional per-release floor inside `versions[]`.
|
||||||
|
|
||||||
|
The gate as merged reads only the floor. Today that is harmless: no manifest
|
||||||
|
uses an upper bound, and the two fields agree everywhere except
|
||||||
|
`7-segment-clock` (`>=1.0.0` against a `2.0.0` floor). But the fields *can*
|
||||||
|
disagree, and the range syntax the schema already permits includes upper bounds
|
||||||
|
— a plugin declaring `["2.0.0 - 2.9.9"]` means "not compatible with 3.x" and
|
||||||
|
the gate would install it on 3.2.0 regardless.
|
||||||
|
|
||||||
|
**Before B6, the gate must evaluate `compatible_versions` as well**, and the
|
||||||
|
manifest migration must reconcile the two fields rather than only renaming the
|
||||||
|
floor. Deciding which wins when they disagree is part of that work; the safe
|
||||||
|
default is the more restrictive.
|
||||||
|
|
||||||
|
(The schema also deprecates a top-level `ledmatrix_version` in favour of
|
||||||
|
`compatible_versions`. No manifest still carries it, so there is nothing to
|
||||||
|
migrate there.)
|
||||||
|
|
||||||
### B5 — adoption is safe by construction
|
### B5 — adoption is safe by construction
|
||||||
|
|
||||||
A plugin adopting core imports keeps its bundled copy and reaches it through the
|
A plugin adopting core imports keeps its bundled copy and reaches it through the
|
||||||
@@ -274,11 +297,29 @@ been in users' hands long enough that the population running a core without it
|
|||||||
is small.** The bundled copies cost disk space; deleting them early costs
|
is small.** The bundled copies cost disk space; deleting them early costs
|
||||||
scoreboards, silently. That trade is not close.
|
scoreboards, silently. That trade is not close.
|
||||||
|
|
||||||
Before the first sunset, add a **compatibility regression test**: load each
|
Before the first sunset, add a **compatibility regression test**. It has to
|
||||||
adopted plugin with its bundled copy removed against a pinned old-core worktree
|
cover four cases, not one — B5's safety claim and B6's failure mode are
|
||||||
and assert it fails loudly and specifically, then against current core and assert
|
different propositions and only the second is obvious:
|
||||||
it works. That test is what turns "we think this is safe" into something CI
|
|
||||||
re-checks on every change.
|
| | bundled copy present | bundled copy removed |
|
||||||
|
|---|---|---|
|
||||||
|
| **pinned old core** | **loads** — this is B5's whole guarantee, that the guarded import falls back | `PluginState.ERROR`, and the recorded error names the exact missing module |
|
||||||
|
| **current core** | loads, using core code | loads, using core code |
|
||||||
|
|
||||||
|
The top-left cell is the one worth writing first: nothing in the suite currently
|
||||||
|
proves that an adopted plugin still works on a core that predates the module,
|
||||||
|
which is the entire basis for saying B5 is safe to run ahead of the gate.
|
||||||
|
|
||||||
|
Assert the old-core/removed-copy case as `PluginState.ERROR` **plus the missing
|
||||||
|
module path**, not as an uncaught exception. `PluginManager.load_plugin` catches
|
||||||
|
`ModuleNotFoundError`, so nothing propagates — a test expecting a raise would
|
||||||
|
pass for the wrong reason on a core where the module is merely broken rather
|
||||||
|
than absent. "Fails loudly" is aspirational, not what the code does today: it
|
||||||
|
fails into `ERROR` state with one log line, which is precisely why B6 needs the
|
||||||
|
gate rather than trusting the failure to be noticed.
|
||||||
|
|
||||||
|
The same suite should exercise the install/update gate, since it is the other
|
||||||
|
half of the guarantee.
|
||||||
|
|
||||||
## What's next
|
## What's next
|
||||||
|
|
||||||
@@ -297,10 +338,14 @@ In order. Each step is independently useful and independently revertible.
|
|||||||
`src.__version__`, and surface the reason in the store UI rather than only
|
`src.__version__`, and surface the reason in the store UI rather than only
|
||||||
the log. This is the single change that turns the floor from documentation
|
the log. This is the single change that turns the floor from documentation
|
||||||
into a guarantee, and B6 depends on it.
|
into a guarantee, and B6 depends on it.
|
||||||
4. **Migrate the manifests** to `ledmatrix_min_version`. Currently 28 plugins
|
4. **Migrate the manifests** to `ledmatrix_min_version`, and reconcile them with
|
||||||
spell it both ways across their `versions[]` entries, 12 use only the old
|
`compatible_versions` (see above — that field is the required, canonical one,
|
||||||
spelling, and 2 only the new. Scope the sweep to the nine sports plugins if a
|
and the gate does not read it yet). Currently 28 plugins spell the floor both
|
||||||
42-plugin version-bump wave isn't worth it.
|
ways across their `versions[]` entries, 12 use only the old spelling, and 2
|
||||||
|
only the new. Scope the sweep to the nine sports plugins if a 42-plugin
|
||||||
|
version-bump wave isn't worth it — but the `compatible_versions` half has to
|
||||||
|
cover every manifest the gate can refuse, or define explicit legacy handling,
|
||||||
|
before the gate is allowed to block anything.
|
||||||
5. **Run B5 adoption** — hockey, soccer, football, then the remaining six.
|
5. **Run B5 adoption** — hockey, soccer, football, then the remaining six.
|
||||||
Bundled copies stay. Byte-identical harness output per plugin, then a soak.
|
Bundled copies stay. Byte-identical harness output per plugin, then a soak.
|
||||||
6. **Only then plan B6**, with the compatibility regression test described above
|
6. **Only then plan B6**, with the compatibility regression test described above
|
||||||
|
|||||||
Reference in New Issue
Block a user