Discord Name:
Axas#7680
GitHub Repository:
Description:
A set of search, retrieval, and other misc plug-ins for Red Discord Bot
Overview
I have reviewed your repository and made a list of changes for each cog. While this list may seem long, the requested changes are pretty minor and easy to fix. You have a good number of cogs, which also added to the length of this review. I included some references for things that may need more explanation, but not everything has a reference link. If you require more information please reply to this response.
I also included some recommendations as well, which are not required for you to pass this review, but some stuff to keep in mind moving forward. Once you have made these changes, a final review will take place. Reply on this thread letting QA know that the changes have been made. If we find any other changes that need to be made, myself or another QA member will let you know as soon as possible.
Corrections (Must fix for approval)
Overall
- The author key in your info.json files need to be a list of strings. Reference
- Your info.json keys in longcat, loot, the100, and lootbox are all caps, but should be all lowercase. Reference
Advanced Google
- Class missing a docstring.
Horoscope
- Cog info.json should disclose that a data folder is bundled and created. Reference
- This cog has the possibility of being I/O heavy with the read/writes in the
file_check
function and should disclosed in the info.json. -
bundled_data_path
should be moved to the package’s__init__.py
setup function prior to loading the cog.
Long Cat
- Class missing a docstring.
- Cog info.json should disclose that a data folder is bundled. Reference
Emote
- Cog info.json should disclose that data is created. Reference
Geico
- Geico shouldn’t be using
bot.reply
, it should usectx.send
. Reference - The
compare
command has some unresolved references with themissing
variable in the else block.
Loot
- Docstring missing for the class.
Loot Box
- Docstring missing for the class.
- Cog class name should be camel case.
- Remove
pass_context=True
in the command signatures.
Points
- Remove
pass_context=True
in the command signatures. - In the
reset
command for Points,bot.wait_for
is incorrect. You need to specify an event. Additionally, the structure for these have changed. It no longer returnsNone
when it times out. It raises anasyncio.TimeoutError
, which must be caught.
Strawpoll
- Doc string missing for the class.
The 100
- Class missing a docstring.
-
perm_check
function should usebot.owner
instead ofbot.settings.owner
. -
role
andtoken
commands are checking for None inbot.wait_for('message'...)
events. Should be
catching the raisedasyncio.TimeoutError
instead.
Trove
- Class missing a docstring.
-
perm_check
in Trove cog should be usingbot.owner
instead ofbot.settings.owner
Recommendations (Not necessary to change for approval)
- horoscope.py should use the bundled data for the data folder instead of using
os.path
- Put a line break between the first and second line on your docstrings. Otherwise the output will look unexpected when using help.
- For your group commands, consider using autohelp for custom, and for the rest remove the unecessary
ctx.invoked_subcommand
. - Consider changing your bare excepts in welcome to
except asyncio.TimeoutError
This is being closed due to inactivity. Please resubmit when you have more time.