[APPROVED] JackCogs

Discord Name: Jackenmen#6607 (User ID: 176070082584248320)

GitHub Repository (Must be V3): https://github.com/jack1142/JackCogs

Description: Cogs requested by others and those that I wanted for Red myself.

Now or never, right? :stuck_out_tongue:

For anyone who will end up assigned to this, DM me on Discord if you want to test rlstats cog, I can supply you with a token for the time of review.

Hi Jack, thank you for your patience while I review your repo. There’s really only one minor issue I have with your repo. Once that’s fixed I’ll be happy to mark this as approved.

Commit hash at time of review - d585125113d0620891972084cd41d5f8a977356e

ModRoles

modroles.py

  • Line 126, : This could be a problem. The bot owner has the ability to give themselves any role lower than the bots role which means a user may get their bot on the server and easily assign themselves the highest role just below the bot. While this is technically possible regardless of this cog it makes it extremely easy to utilize this capability without relying on knowing how to code. I’m marking this as an optional change and more something to consider. Optional

NitroRole

nitrorole.py

  • Lines 4 and 5: You import Template, Any, and Iterable but never use them. Optional
  • Line 262: This is unnecessarily long, guild.get_channel returns None if the provided input is invalid. Optional

VoiceTools

voicetools.py

  • Lines 86 to 94: If a user in the ignore list leaves the server or any ignored roles or voice channels are delete this will error out.

I’m leaving this as is, you’re right it makes it really easy to assign yourself a role just below bot’s role for bot owner, but bot owner can generally do a lot, so doesn’t seem that harmful

Fixed, thank you for noticing this.

I’m currently doing this to keep mypy happy - type hint doesn’t allow None to be passed so it gives me incompatible type error. With that said though, I might look into modifying this later.

Wow, I don’t know how I missed that one… Should be fixed now (including occurrence in the other list command in the cog), thank you for noticing this :slight_smile:

Thanks for making those changes I’m marking this as approved.