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

Create cog disabling API #4043

Merged
merged 20 commits into from
Jul 28, 2020
Merged

Create cog disabling API #4043

merged 20 commits into from
Jul 28, 2020

Conversation

mikeshardmind
Copy link
Contributor

@mikeshardmind mikeshardmind commented Jul 6, 2020

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

Allow disabling cogs on a per guild basis (resolves #3945)

@mikeshardmind mikeshardmind changed the title create cog disbale base Create cog disabling API Jul 6, 2020
@mikeshardmind mikeshardmind marked this pull request as ready for review July 7, 2020 00:23
@mikeshardmind
Copy link
Contributor Author

This should be ready to go now, though I won't be able to test the changes personally on this one until slightly later (can update this when that's happened unless others are interested in confirming this before I do)

@mikeshardmind
Copy link
Contributor Author

Tested, updated, tested again and working as intended.

@Jackenmen Jackenmen added Category: Bot Core Status: Needs Discussion Needs more discussion. Type: Feature New feature or request. labels Jul 7, 2020
@Jackenmen Jackenmen added this to the Backlog milestone Jul 7, 2020
@mikeshardmind
Copy link
Contributor Author

Updated for settable default state per discord discussion. Current state of PR is untested.

@Jackenmen Jackenmen modified the milestones: Backlog, 3.3.11 Jul 20, 2020
@Jackenmen Jackenmen removed the Status: Needs Discussion Needs more discussion. label Jul 20, 2020
@mikeshardmind
Copy link
Contributor Author

Rebased over the other changes to announcer made in #4089 , + uses the correct cog name

@mikeshardmind
Copy link
Contributor Author

Reiterating something from before: anything after the initial commit has not been personally tested by me yet.

The initial commit (with message create cog disbale base [sic]) has been tested, but the changes made to accommodate a bot owner defaulting a cog to disabled has not yet (this is something I intend to handle this evening if time permits)

@Jackenmen Jackenmen self-assigned this Jul 27, 2020
Michael H added 3 commits July 27, 2020 21:15
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Streams cog should check if cog has been disabled in the guild it's checking stream alerts for.
  2. (I am not entirely sure about this, but it's probably the right thing to do) Tempban expiration shouldn't happen for guilds that disabled the Mod cog.
  3. I want @Drapersniper or @aikaterna to sign off on how this is used in Audio (I haven't checked that myself at all)

Everything else is in review comments.

docs/guide_cog_creation.rst Outdated Show resolved Hide resolved
redbot/cogs/alias/alias.py Show resolved Hide resolved
redbot/core/core_commands.py Outdated Show resolved Hide resolved
redbot/core/core_commands.py Outdated Show resolved Hide resolved
redbot/core/core_commands.py Outdated Show resolved Hide resolved
redbot/core/core_commands.py Outdated Show resolved Hide resolved
redbot/core/core_commands.py Outdated Show resolved Hide resolved
redbot/cogs/admin/announcer.py Show resolved Hide resolved
redbot/cogs/alias/alias.py Show resolved Hide resolved
redbot/cogs/reports/reports.py Outdated Show resolved Hide resolved
@mikeshardmind
Copy link
Contributor Author

1. Streams cog should check if cog has been disabled in the guild it's checking stream alerts for.

2. (I am not entirely sure about this, but it's probably the right thing to do) Tempban expiration shouldn't happen for guilds that disabled the Mod cog.

3. I want @Drapersniper or @aikaterna to sign off on how this is used in Audio (I haven't checked that myself at all)

Everything else is in review comments.

1 & 2 are handled, as are the review comments.

As far as I could tell, everything in audio should be initiated by a command, the events won't be triggered without a corresponding command.

Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the review of Audio parts, there's only one review comment left.

redbot/cogs/reports/reports.py Outdated Show resolved Hide resolved
@Drapersniper
Copy link
Contributor

just a quick FYI, there's a few events missing the check, Ill rewier it properly later today and note them down then.

Jackenmen
Jackenmen previously approved these changes Jul 28, 2020
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Just added 2 small code suggestion for strings that you can just merge

redbot/core/core_commands.py Outdated Show resolved Hide resolved
redbot/core/core_commands.py Outdated Show resolved Hide resolved
@Jackenmen Jackenmen added the QA: Passed Used by few QA members. Has been approved by the assigned QA member(s). label Jul 28, 2020
@Jackenmen Jackenmen merged commit 1d80fe9 into Cog-Creators:V3/develop Jul 28, 2020
@Cog-CreatorsBot Cog-CreatorsBot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Jul 28, 2020
@Jackenmen Jackenmen added Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. and removed Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. labels Jul 29, 2020
@Jackenmen Jackenmen modified the milestones: 3.3.11, 3.4.0 Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. QA: Passed Used by few QA members. Has been approved by the assigned QA member(s). Type: Feature New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core] Add an API to toggle cogs on a per server level
4 participants