[APPROVED] Trusty-cogs

Applications
#1

Discord Name:
TrustyJAID#0001
GitHub Repository (Must be V3):


Description:
A variety of utility cogs and fun cogs including Hockey information from the NHL, badge creation from discord user information, starboard for use on Redbot, twitch follow notifications and more.

#2

Thanks for your submission TrustyJAID. I’m currently in the process of reviewing your repository, but due to it’s size (40+ cogs), it will take some additional time to go through everything. Just wanted to let you know that, a review has been started, but it will take longer than normal. Stay tuned!

1 Like
#3

This is a long overdue review of your repo. It’s been a shame that I haven’t finished it until now. I was maybe halfway through a review a long time ago, but you fixed a bunch of the things I was going to bring up, and the commit history is different so I had to start over lol.

Most of the items that need changing is minor housekeeping things such as missing docstrings. A couple of your cogs have conflicts with the core, not sure if the cogs predate the functionality being added to core or if they extend it. My list looks lengthy, but it’s only because you have so many cogs! I think you will fine this assessment fair and should be easy to fix. If you have any questions please let me know here.

** Add Image**

  • Because Add Image creates folders and saves/deletes images you need to disclose that information in this cog’s info.json install_msg.

Anime

  • Anime main class is missing a doc string for help

  • anime reset command is missing a doc string for help

  • anime airing command is missing a doc string for help

  • anime search command is missing a doc string for help

Auto Role

  • In the role command, you need to add an additional check to make sure that the person adding an auto assign role, isn’t able to add a role higher than their own.

Backup

  • You need to specify in the installation message of this cog, that a data folder will be created to hold the backup and that it is saved as a JSON.

Barcode

  • Add a note in your installation msg that this cog comes bundled with a data folder to hold fonts.

CleverBot

  • Line 225 should be channel.send instead of ctx.send

Dev

* This cog seems to have conflicts with multiple core commands in Red. Just let me know the rationale on this cog, nothing else seems to be wrong. (This is fine)

ExtendedModLog

  • You may have an unresolved reference with the perp variable, when the statement: if before_attr != after_attr: is False.

Faces

  • Indicate that there is a data folder that comes bundled with some data in the installation msg.

Fun

  • Not a huge deal, but you have a few methods, like has_dupe, that are being called directly by using Fun.has_dupe. I recommend converting those to static methods with the @staticmethod. This is optional.

Halo

  • Missing an import for asyncio

Hue

  • Missing ctx in the command hue_connect
  • Missing ctx in the function oilersgoal

ImageMaker

  • Disclose the data folder and the contents

Mock

mock command has a name conflict with the core cog dev. (This is fine)

NotSoBot

  • You have two keys named zero in your self.emojis dictionary. (Optional)
  • In your rotate command, you commented out your scale variable declaration, which results in an unresolved reference for your ctx.send
  • You need to disclose the bundled data with this cog.

QPosts

  • Missing a doc string for the command reset_qpost
  • Missing a doc string for the command dlq

Reports Pin

  • Class Reports Missing a doc string
  • Recommend changing the class name to ReportsPin to align with the cog name. (Optional)

Retrigger

  • Disclose in the installation msg that this cog will create a folder, and that it saves images to it.
  • In the trigger class, avoid setting the default values to something mutable. Just set the instance variable to the empty list or dictionary instead of in the function signature for __init__.

StarBoard

  • Command `whitelist_add missing a reference to role in like 386 when appending the role id to the whitelist

TrustyAvatar

  • Missing a docstring for check status
  • Recommend in the install message to warn users that changing the interval puts their bot at risk of rate limiting (Optional)

Twitch
* Name conflict with twitch command and core cog Streams's twitch group command. (This is fine)

Welcome

  • Missing an import for asyncio
#4

Hey, thank you for taking the time to go through this. I have completed all the changes you recommended except for the Twitch one. I’m not sure the best way to proceed since there is no name conflict in loading both cogs on the latest core bot as the Twitch cog uses twitchhelp|t|twi for its command group and the Streams cog uses twitch which I would have liked to use but didn’t want to interfere with the already functioning Streams cog.

#5

Edited my response. Your twitch cog is fine. That was my mistake.

#6

Awesome, all the changes you recommended should be found here :https://github.com/TrustyJAID/Trusty-cogs/commit/2ac5fbb52e19e1edb48a827d1257ffcf0f42f954
I didn’t change the name of the reportspin class since I was only asked if I could make reports pinned and I didn’t want to mess with other servers setup to use the reports cog. I know I can still access the config but that could cause issues if anyone else tries to use the cog on its own which I have set to hidden because it’s more of a specialty case for a few servers my bot is on.

1 Like
#7

Thanks. Marking this as approved.

1 Like
#8

I am glad this ends with a happy ending

1 Like
#9
#10
#11
#12