-
-
Notifications
You must be signed in to change notification settings - Fork 189
noice: message, notify, health options #1260
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
base: main
Are you sure you want to change the base?
Conversation
|
I'm not opposed to merging, but we've generally agree to avoid overpopulating setupOpts since it's already freeform, and adding more options also adds more maintenance burden inbetween plugin updates. @horriblename @Soliprem wdyt? should we cut poor midi 👨🏻🦲 some slack here? |
|
Merging sounds good to me. On another (OT) note, I was thinking that linking the docs for plugins we define the freeform setupOpts for might be a good idea, as a thing for the future. |
|
If we resort to freeform setupOpts in the future, shouldn't we just pull the plug then? Seems like early feature block considering that the freeform hasn't been implemented yet. |
What do you mean? I don't really think this should be merged due to reasons NotAShelf mentioned |
|
If we're going for the freeform route then we should actually remove the existing setOpts options besides what's necessary or being overwritten? |
|
mkPluginSetupOption is freeform tho |
|
@horriblename Yes, but before making this PR there were still a few unneeded options created such as presets. Shouldn't these be removed in preference of free form style as well? To the same line of reasoning, these too seem unnecessary. |
Provides message, notify, and health options to the
noiceplugin.Sanity Checking
nix fmt).#nix(default package).#maximal.#docs-html(manual, must build).#docs-linkcheck(optional, please build if adding links)x86_64-linuxaarch64-linuxx86_64-darwinaarch64-darwinAdd a 👍 reaction to pull requests you find important.