Skip to content
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

[Streams] Save/Remove/Check custom messages for each streamer. #6483

Open
wants to merge 16 commits into
base: V3/develop
Choose a base branch
from

Conversation

SharkyIsAKing
Copy link

Type

  • BugFix
  • Enhancement
  • New feature

Description of the changes

This is originally from @KianBral & @willkang-1, since the last PR has been idle for so long I decided to bring this back and work on it. This will close #4668, resolving #3824 and fixes #4984

This PR has not been tested, I have merged the previous authors' commits and work, while also slowly going through #4668 comments to handle the requested changes.

Have the changes in this PR been tested?

No

@github-actions github-actions bot added the Category: Cogs - Streams This is related to the Streams cog. label Nov 28, 2024
@Flame442 Flame442 added the Type: Feature New feature or request. label Nov 28, 2024
@Flame442
Copy link
Member

Unless I am misreading the diff at a glance, it looks like your commands (check/streamer/remove) are duplicated. I have a minor nitpick with the function names not being in group_sub format, but it looks like the cog already wasn't following that standard so I don't know if it's worth worrying about.

For UX consideration - do we want there to be 3 commands for setting/checking/removing, or do we want to merge check into streamer as the behavior when no argument is passed? (Or remove, I think there is precedent in other parts of the bot for either)

Additionally, you are currently failing the style checks. Please format your PR with black -l 99 using the instructions in our CONTRIBUTING.MD file.

@SharkyIsAKing
Copy link
Author

Honestly I prefer group standards as well, the only things I've done so far are transfer the commits to keep the authors, make sure the .formats suppose i18n and adjust one of the things mentioned in the previous PR.

The rest I've not touched, personally I'd agree with merging and having them as a group_sub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Cogs - Streams This is related to the Streams cog. Type: Feature New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Streams] Ability to utiltize more role mentions
3 participants