-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Switch to out of tree AudioReactive Usermod #5311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
netmindz
wants to merge
14
commits into
wled:main
Choose a base branch
from
netmindz:AR-Usermod-out-of-tree
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+117
−2,964
Open
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
0e15bf0
Remove in-tree audioreactive
netmindz 4df67e0
Use MoonModules/WLED-AudioReactive-Usermod
netmindz 5ad2287
custom_usermods is for in-tree only
netmindz d1aba86
Pull new ERR_ values into upstream
netmindz a489170
define library as name url combo
netmindz ea78f51
oappendi
netmindz e8ee8c0
Add missing javascript function from WLED-MM
netmindz 7e4fac8
Add missing javascript function from WLED-MM
netmindz 1f6a375
Revert "define library as name url combo"
netmindz 8a34925
Revert "custom_usermods is for in-tree only"
netmindz c23f6f2
Swap to using ${common.default_usermods} and new style syntax for out…
netmindz 676b56b
Revert "Use MoonModules/WLED-AudioReactive-Usermod"
netmindz 1dd7edd
Merge branch 'main' into AR-Usermod-out-of-tree
netmindz fd7fa1a
Merge remote-tracking branch 'AC/main' into AR-Usermod-out-of-tree
netmindz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: wled/WLED
Length of output: 67
🌐 Web query:
WLED-AudioReactive-Usermod GitHub releases versions MoonModules💡 Result:
MoonModules (WLED-MM) releases (GitHub Releases):
Citations:
🏁 Script executed:
Repository: wled/WLED
Length of output: 2421
🏁 Script executed:
Repository: wled/WLED
Length of output: 4387
Specify a commit hash or branch for the AudioReactive library to match the version pinning pattern used throughout platformio.ini.
All other dependencies in this file pin to specific versions, commits, or tags (e.g.,
fastled/FastLED @ 3.6.0,https://github.com/Aircoookie/GifDecoder.git#bc3af189b6b1e06946569f6b4287f0b79a860f8e). The AR_lib_deps lacks any pinning, making builds non-reproducible and inconsistent with the file's standards.Example with commit hash
(Also remove the extra space after the first
=.)🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@netmindz, that makes perfect sense! During parallel development when both repositories are evolving together, pinning to a specific commit would indeed create unnecessary friction and make testing difficult.For the active development phase, keeping the dependency unpinned is the pragmatic approach. However, I'd recommend pinning to a specific commit hash before this PR is merged to
main—that way, the release builds remain reproducible and users get a tested, stable version of the AudioReactive usermod.Feel free to update the pinning when you're ready to finalize the PR. Thanks for the clarification! 👍
🧠 Learnings used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai Yes you are correct, this is the pattern we use, however this PR is going side by side with the creation of that repo, so we don't want to keep updating this PR after every commit to the usermod, as the only way to test the usermod is through it's usage here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@netmindz, understood! I see you're confirming the parallel development approach. That makes complete sense for your current workflow—keeping it unpinned allows you to iterate freely while both repositories evolve together.As mentioned earlier, feel free to pin to a specific commit when you're ready to finalize before merging to
main. No action needed for now. 👍🧠 Learnings used