docs(christmas-countdown): document every setting, including the five that do nothing - #379
Open
ChuckBuilds wants to merge 1 commit into
Open
docs(christmas-countdown): document every setting, including the five that do nothing#379ChuckBuilds wants to merge 1 commit into
ChuckBuilds wants to merge 1 commit into
Conversation
… that do nothing Documentation only; no behaviour change. Half this plugin's configuration has no effect, and the README now says so rather than describing what the schema promises. The four transition settings -- type, speed, enabled, and high_performance_transitions -- appear nowhere in manager.py, and the core implements no display transitions either; every "transition" under LEDMatrix/src is a comment about plugin state machines or Vegas cycle boundaries. tree_size is worse: it is read and validated, so a value of zero is rejected with a warning, and then never applied. Rendering at 8, 16, 24, 32 and 48 gives one hash. Those are tracked as #377 rather than removed here. Six named transition types reads like deliberate design waiting on implementation rather than an accident, and removing five of ten settings is the maintainer's call. The layout description is corrected. The old README said the tree is centred with the countdown text below it; the code splits the panel down the middle and puts the tree left, text right, at every size -- which the panel-size images show. Also records that tree_color applies only when the bundled tree image is missing, so on a normal install it does nothing visible. Requirements and testing sections are restored from the old file, and check_plugin is documented alongside the emulator since this plugin ships harness goldens. Not fixed here, filed as #378: golden drift on seven of eight sizes, pre-existing on main with identical figures when these changes are stashed. That is the third plugin with drift after of-the-day (#371) and geochron (#375); if all three pass in a known-good environment the common cause is likely a dependency version rather than three regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
|
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.
Twelfth plugin in the README pass. Documentation only; no behaviour change.
Half this plugin's configuration has no effect
That's the headline. Five of ten settings do nothing, and the README now says so plainly instead of describing what the schema promises:
transition.type(6-value enum)transitionappears nowhere inmanager.pytransition.speedtransition.enabledhigh_performance_transitionshigh_performanceappears nowhere eithertree_sizeI checked the core before concluding, in case transitions were a display-controller feature the plugin merely declared — they aren't. Every "transition" under
LEDMatrix/src/is a comment about plugin state machines or Vegas cycle boundaries.tree_sizeis the nastiest of the five, because it validates: set it to-5and you get a warning, which is fair evidence to anyone testing that the setting is live. Then rendering at 8, 16, 24, 32 and 48 gives a single hash:Tracked as #377 rather than removed here — six named transition types reads like design waiting on implementation rather than an accident, and dropping five of ten settings is your call. I'll do the removal if you'd prefer it.
A layout correction
The old README said the tree is "centered horizontally and positioned in the upper portion" with "countdown text centered below the tree". The code splits the panel down the middle and puts tree left, text right, at every size:
The panel-size images show it. Also recorded:
tree_colorapplies only when the bundled tree PNG is missing, so on a normal install it does nothing visible — the schema says this, but "green tree colour" reads like it should work.Golden drift, filed as #378
Pre-existing on
main, identical figures with my changes stashed. Not regenerated, same reasoning as before.This is the third plugin with drift — of-the-day (#371), geochron (#375), and this one. If all three pass in your environment, the common cause is more likely a dependency version than three separate regressions. This plugin scales a PNG with
LANCZOS, which is exactly the kind of operation that can differ between Pillow releases. Might be worth recording the version the goldens were generated against somewhere the harness can check.Audits
--checkimages match🤖 Generated with Claude Code