-
Notifications
You must be signed in to change notification settings - Fork 17
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
Simplify and clarify config #40
base: main
Are you sure you want to change the base?
Simplify and clarify config #40
Conversation
whoops, i clicked the wrong button 😓 |
I think I'm gonna stick with
I'm not sure renaming
Moving accounts above crons makes sense, and I agree. However, I'm not sold on ditching the whitelist/blacklist for crons just yet. It's super easy for users to quickly enable or disable crons that way. Your "enabled" flag idea works too, and I think it would be nice, but it might be more complicated for new or current users. But I'm all ears for suggestions.
Letting users split notifications per account using different webhooks, like I said above, is a really neat idea! I'm just not sure about the best way to set that up in the config yet. Maybe we could add an array to each webhook token? That would mean reworking how the platform methods handle things though |
Mostly personal preference,
Also a matter of personal preference, I think
I strongly believe having an
I hadn't yet looked at the implementation, and have been focusing on optimizing the config file as much as I could. I figured looping over the integrations on start-up and spinning up a module that runs an instance of the required integration would be feasible. I have not yet gone through the effort of implementing any of these changes, as I wanted to discuss the draft first. |
This is a draft PR that I've been working on to suggest a simplified and clarified configuration structure. I'd love to discuss these options and see if we can move ahead with any or even all of these changes. This PR is not meant to be directly implemented as is and needs lots of work to be fully functional.
Overview
camelCase
tosnake_case
as I believe this to be easier to read.platforms
section was renamed tointegrations
as I believe this is a clearer description of the contained data.integrations
section can now contain a multitude of different types of integrations to facilitate splitting notifications and commands.crons
section was moved down below theaccounts
section, as users are less likely to want to change crons, whereas theaccounts
section I believe to be a higher priority.accounts
section was completely revamped. Each account contains a single cookie and can be configured per game, instead of having a separate cookie for each game for easier maintainability.accounts
section now contain anintegrations
section which contains an array of integrations to use in accordance with the previously specifiedintegrations
entries. For example, this would allow users to split Discord Webhook notifications for two separate accounts between two separate channels.crons
section no longer contains awhitelist
andblacklist
entry.crons
entries are now objects with an explicitenabled
flag, which I believe to be clearer than a blacklist and whitelist system.