feat(sports): share the card helpers the eight scoreboards each carried - #513
Conversation
Twenty methods were byte-identical in all eight scoreboards' game_renderer.py: the colour pickers, the scroll_card settings lookup, the date and time formatting, the favourite-team rules and the font-size grid snapping. Every fix to any of it had to be made eight times, and a new scoreboard began by copying them a ninth. src/common/sports_card.py holds them once. 245 lines leave each plugin. **Free functions, not a base class.** Every helper takes config/logger/fonts as arguments rather than reading them off an instance, so a plugin keeps its method and delegates the body -- call sites, signatures and override points are all untouched. Adoption is therefore per-function and reversible, which is what let all eight move with byte-identical renders. The bodies are the plugins' code moved, not rewritten. Two deliberate differences, both verified: - crisp_size takes the seven-plugin guard (`not desired`) rather than football's. They agree on every real input; the extra guard only stops a None size raising TypeError, so adopting it is a no-op for seven plugins and removes a crash path for the eighth. - schema_font_size caches per schema PATH. The plugins cached on their own class, which is the same distinction expressed without a class to hang it on; two plugins never share an entry. The path has to be passed in because the plugins derived it from __file__, and __file__ here is the core's. Verified before any plugin was touched: 534 differential comparisons of the helpers against afl's originals and 704 more of the font-sizing chain against all eight plugins' originals -- 1,238 comparisons, zero differences. Writing the constant tables by hand introduced two errors that check caught: the tie colour was (255,255,0) instead of the plugins' (255,200,0), and a "five_by_seven" alias that does not exist. Both are now taken from the plugins verbatim. 43 tests pin the contract, including the cases the plugins' own comments record as having bitten: a three-character string must not iterate into a colour, a shared font face must give up rather than guess an element, a font_size equal to the schema default carries no intent, and a bad timezone falls back to UTC rather than blanking the card. Full suite 3768 passed, 6 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
|
Warning Review limit reachedNext included review available in 43 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
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 | 124 |
| Duplication | 0 |
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.
|
@coderabbitai review |
|
|
@coderabbitai review |
|
1 participant
Twenty methods were byte-identical in all eight scoreboards'
game_renderer.py: the colour pickers, thescroll_cardsettings lookup, the date and time formatting, the favourite-team rules and the font-size grid snapping. Every fix to any of it had to be made eight times, and a new scoreboard began by copying them a ninth.src/common/sports_card.pyholds them once. 245 lines leave each plugin (companion plugins PR).Free functions, not a base class
Every helper takes
config/logger/fontsas arguments rather than reading them off an instance, so a plugin keeps its method and delegates the body — call sites, signatures and override points all untouched. Adoption is per-function and reversible, which is what let all eight move with byte-identical renders.The bodies are the plugins' code moved, not rewritten. Two deliberate differences, both verified:
crisp_sizetakes the seven-plugin guard (not desired) rather than football's. They agree on every real input; the extra guard only stops aNonesize raisingTypeError— so it is a no-op for seven plugins and removes a crash path for the eighth.schema_font_sizecaches per schema path. The plugins cached on their own class; the path is that same distinction without a class to hang it on, and two plugins never share an entry. It must be passed in because the plugins derived it from__file__, and__file__here is the core's.Verified before any plugin was touched
1,238 differential comparisons against the plugins' own originals — zero differences. 534 of the helpers against afl's, and 704 of the font-sizing chain against all eight plugins' (including football, whose
_crisp_sizevariant this replaces).That check earned its keep: writing the constant tables by hand introduced two silent errors it caught — the tie colour was
(255,255,0)instead of the plugins'(255,200,0), and I had invented afive_by_sevenalias that does not exist. Both now come from the plugins verbatim, and all six tables are asserted equal.Tests
43 tests pin the contract, including the cases the plugins' own comments record as having bitten:
font_sizeequal to the schema default carries no intent (the web UI writes the whole default block on every save)Full suite: 3768 passed, 6 skipped.
Sequencing
This is a pure addition — nothing imports it yet, so it is safe to merge on its own. The companion plugins PR floors at 3.3.0 and must not merge until that release is cut.
🤖 Generated with Claude Code
https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9