VSCogs - Application Review
Thank you for your patience whilst I conducted a review of your repository. Please read the comments below about your sole cog, VSMod. If you have any queries during the review, please don’t hesitate to contact me on Discord @ kreusada. You can find me in the Red server.
General Comments
Print Statements
There are 10 print statements which appear to “log” information to the console. These can be found on lines 72, 479, 542, 544, 548, 550, 554, 556, 1095 and 1116. Print statements are not a substitute for proper logging. Neither is your debug_log method. You should import logging (which you’ve already done, but haven’t used) and instantiate your own logger with our conventional identifier format (logging.getLogger("red.your_repo_name.cog_name")).
Moreover, with the current way that you are essentially logging to a separate file, it is using more processing power and is logging unnecessary detail. Command execution typically doesn’t need to be logged, and if it needs to be, it shouldn’t be handled in this way.
Prefix Representation
There are cases where you show command invocation examples using the ! prefix. This would not be suitable for approved cogs, as other bots will have differing prefixes. You should use ctx.prefix/ctx.clean_prefix, or [p] inside doc-strings.
Requests Usage
The first thing I noticed was your use of the requests library. The requests library is blocking because it operates synchronously, meaning it halts the entire program’s execution while waiting for a response from the server, which can cause delays in an asynchronous environment like an operating Red instance. Instead, you should use aiohttp and instantiate a ClientSession which is bound to your cog’s class. It should also be gracefully closed on cog_unload.
Convoluted on_command_error Method
The on_command_error listener is extremely convoluted, repetitive, and is low quality. Moreover, these checks are redundant as they are already handled gracefully by Red and d.py internals. I would recommend removing this method and only using it if necessary.
Command and Cog Doc-Strings
It is a requirement that every command, and the cog itself, has a doc-string. Unfortunately, no doc-strings are currently present so the help menu would be uninformative for other users.
Config Calls
In your on_message listener, you are making an excessive number of calls to Config, with some settings being called more than once for every action setting. You should avoid repetitive calls in general, however, we can make use of Group.all() to grab the config of the given scope all at once as a dictionary. This is more efficient than multiple calls to Config. For example, await self.config.guild(message.guild).all().
Refactoring on_message
As stated above, there are many repeated calls and conditional statements. This method should be cleaned up to avoid overloading of the bot. Remember, on_message is called for every message sent. If the initial checks are passed, the subsequent checks are ran for every message. They should be optimised as they are called frequently.
Overwriting Core Cog Commands
Commands from Core cogs (i.e. kick, mute, warn) share the identifiers with some of the commands from this cog. There is no evidence that this is handled gracefully, or there is no warning that warnings, mutes, mod, and any other conflicting cog should not be loaded whilst VSMod is.
Permissions
In some cases, you do not check if the bot has sufficient permissions to perform an action on a user. For example, kicking a user in the kick command. The decorator checks whether the author is a mod or has ban_members permission (which, should be changed for kick), but it doesn’t check if the bot has sufficient permissions. Moreover, you attempt to send a message to the user’s DM, but this will error if the user has their DMs turned off.
Deprecation: checks
The checks module is deprecated, permission decorators should now be inherited from the commands module.
Maintainability
Recommendation: You should consider structuring your code in a way that eases the job for you when it comes to making updates. With all your code in one file, no doc-strings and no type hinting, it makes maintainability very difficult, which may create a challenge in complying to future Red updates or bugs/loopholes that may arise from your code.
Decision
Unfortunately, after considering the points raised in this review, I will be rejecting this application. You’ve made good use of Red’s internal API, and the variety of features you’ve implemented shows a strong grasp of the framework. However, there are several highlighted areas that need refinement to meet our standards for approval.
Please consider reading our guide for developers who wish to submit their repositories for approval status. Compliance with more of these points may stand you in good stead for your next application if you wish to re-apply.
Feel free to submit a pull request to our indexing repository if you wish to submit your repository as unapproved. Unapproved cogs do not go through the formal review process and are used at one’s peril.