feat(scoreboards): inherit the shared sports.py logic from the core - #366
Merged
Conversation
Forty-five method bodies in each of these eight sports.py files were byte-identical to the same forty-five in every other scoreboard: 1,007 lines per plugin, 8,056 duplicated in total. They now come from src.common.sports_shared (ChuckBuilds/LEDMatrix#515) and each class inherits the matching mixin. 8,321 lines removed. Three of the forty-eight identical bodies stayed behind on purpose, because a byte-identical body is not automatically safe to move: - _get_timezone binds resolve_timezone from a per-plugin module (hockey_timezone, soccer_timezone, ...). All eight of those files differ -- each carries its own _WRITEBACK_FIXED_IN -- so hoisting the caller would have silently bound every scoreboard to one plugin's copy. - _extract_game_details and _fetch_data are @AbstractMethod stubs. They are the sport contract; satisfying them from a mixin would let a plugin instantiate without implementing its own sport. Class constants are left in place rather than removed, so each plugin's own values still shadow the mixin's defaults. That matters for afl and basketball, which set _SCORE_PROBE_TEXT to "000-000"; the mixin's "00-00" is a default for future plugins, not a change to these. Test changes, all of them making tests exercise the real module rather than a stub: - Nineteen tests stubbed "src"/"src.common" as plain ModuleTypes, so the new import failed with "'src.common' is not a package". They now give those stubs a __path__ into the core, which lets genuine submodules resolve while the stubbed ones stay stubbed. Stubbing the mixins instead would have made every one of those tests pass against dummies -- which is how B5 shipped four of eight broken with every gate green. - soccer/test_schedule_horizon.py AST-parses sports.py looking for _get_weeks_data, which now lives in the core. It looks in both places; its assertions are about the body, which moved verbatim. Verification: all 176 safety-harness renders byte-identical to pristine main, 245 plugin tests pass, five repo gates pass. The one remaining fleet failure is 7-segment-clock/test_render_polarity.py, which fails identically on main and is fixed separately in #364. Versions are bumped above the sports_card/geometry branch rather than above main, since that branch lands first and already claims the next minor.
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 135 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
main gained the odds_text setting (#402) and the card-helper delegation (#359), which together bumped every scoreboard past the numbers this branch claimed. All eight manifests conflicted for that reason alone; no code conflicted. Versions recomputed above main's: afl 1.22.0, baseball 1.40.0, basketball 1.29.0, football 3.4.0, hockey 1.25.0, lacrosse 1.24.0, nrl 1.21.0, soccer 2.24.0. plugins.json regenerated. Checked that the three layers compose rather than assuming it. #402 added an odds font slot and _odds_color() to the same game_renderer.py files #359 delegates twenty methods from, and this branch removes forty-five more methods from sports.py next to them. All three are present together in every plugin: the sports_shared mixin, the sports_card delegations, and the odds wiring. Re-verified against main at the #359 merge: all 176 safety-harness renders byte-identical. The previous CI failure on this branch was stale, not a defect. It ran on 2026-09-02 20:17, and #515 -- which puts src/common/sports_shared.py on core main -- merged 2026-09-03 17:29, twenty-one hours later. The workflow checks the core out at CORE_REF: main, so the module simply did not exist yet when that run imported it.
main gained #403, which adds odds_text to x-propertyOrder in all eight scoreboards and bumps each a patch level. That is the only reason the manifests conflicted again; no code conflicted. Versions recomputed above main's: afl 1.22.0, baseball 1.40.0, basketball 1.29.0, football 3.4.0, hockey 1.25.0, lacrosse 1.24.0, nrl 1.21.0, soccer 2.24.0. Verified all four layers coexist rather than assuming it -- the sports_shared mixin, #359's sports_card delegations, #402's odds wiring, and #403's x-propertyOrder entry are present together in every plugin. Four changes have now landed on the same files this branch edits, so composition is the thing most likely to break silently. test_property_order_coverage.py, the guard that was failing this branch, now passes: it was #402's omission, fixed by #403, not a defect here.
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.
Phase 3 adoption. Needs core ChuckBuilds/LEDMatrix#515. Independent of #359 (
sports.pyandgame_renderer.pydon't overlap), but versions are bumped above #359's since that lands first.What changes
45 method bodies in each of these eight
sports.pyfiles were byte-identical to the same 45 in every other scoreboard — 1,007 lines per plugin, 8,056 duplicated. They now come fromsrc.common.sports_sharedand each class inherits the matching mixin.8,321 lines removed.
Three identical bodies deliberately stayed behind
A byte-identical body is not automatically safe to move — it can bind a module-level name that differs per plugin.
_get_timezonebindsresolve_timezonefrom a per-plugin module (hockey_timezone,soccer_timezone, …). All eight of those files differ — each carries its own_WRITEBACK_FIXED_IN— so hoisting the caller would have silently bound every scoreboard to one plugin's copy._extract_game_details/_fetch_dataare@abstractmethodstubs — the sport contract.Class constants are left in place, so each plugin's own values still shadow the mixin's defaults. That matters for afl and basketball, which set
_SCORE_PROBE_TEXTto"000-000".Test changes — all making tests exercise the real module, not a stub
Nineteen tests stubbed
src/src.commonas plainModuleTypes, so the new import failed with'src.common' is not a package. They now give those stubs a__path__into the core, letting genuine submodules resolve while the stubbed ones stay stubbed.Stubbing the mixins instead would have made all nineteen pass against dummies — which is exactly how B5 shipped four of eight scoreboards broken with every gate green.
soccer/test_schedule_horizon.pyAST-parsessports.pyfor_get_weeks_data, which now lives in the core; it looks in both places, and its assertions are about the body, which moved verbatim.Verification
mainThe one remaining fleet failure is
7-segment-clock/test_render_polarity.py, which fails identically onmainand is fixed separately in #364.One thing worth knowing
_schema_font_sizelocated the plugin'sconfig_schema.jsonvia__file__, which breaks once the method lives in the core — measured at 81% anti-aliased edges instead of pixel-crisp. The 176 renders did not catch it;test_fonts_are_crisp.pydid. Renders alone were not a sufficient gate for this change.