[REJECTED] Npc Cogs

Applications
#1

Discord Name:epic guy#0715

GitHub Repository: https://github.com/npc203/npc-cogs

Description: At the time of applying it’s just 2 cogs, but has a some nice fun commands, both of the cogs are completely original ideas (upto my knowledge), kindly let me know what requirements must I satisfy for the approval

#2
#3

Hi epic guy#0715, thank you for your patience while I review your repo.

Commit hash at time of review - 8f36752b0d6a11fb8b6628980474d4b85fe1ab08

speak

info.json

  • You need to explain that this cog comes with bundled data.
  • install_msg is missing from info.json. This should explain that the cog utilizes webhooks in order to work.

todo

todo.py

  • Line 2: discord is imported but never used. Also it’s generally considered best practice to put different imports on new lines. (Optional)
  • Line 89: This should be await self.config.user(ctx.author).todos.clear() or even better await self.config.user_from_id(user_id).clear() to ensure the default is used when starting again and reduce overall filesize in json without having leftover user ID’s. (Optional)
  • Line 98: This is as simple as await self.config.user_from_id(user_id).clear() and must be done if you have this function in the cog. Having this function inside the cog holding potential user information like this is just hiding the fact this cog stores user information when asked to be deleted when it actually wasn’t.

typeracer

info.json

  • install_msg is missing from info.json. This should explain that text is coming from an external API.

typerace.py

  • Line 85: This should be commands.BucketType.default to restrict it to 1 usage globally. Currently your code only allows for 1 instance of typeracer to run at 1 time. This being set to user means a user can only run it once per user meaning the next user to run it will erase part of the previous users data complicating responses.
  • Line 88: You could utilize a dictionary lookup for the tasks instead allowing more than one person to use this across multiple servers on the bot. I don’t see any other reason to restrict concurrency to 1.
  • Line 30-53: No attribution or credits for this levenshtein calculator. (https://www.datacamp.com/community/tutorials/fuzzy-string-python and https://gist.github.com/Adrianval96/c931555796b95128401d34e9cf74159a would have been good to include if that’s where this is from which it appears to be) A note about this, there’s packages available that can do the levenshtein calculation for you without requiring this, however, taking code like this is not allowed and will result in rejected application.
  • Line 135: Command typer stop has no docstrings.

weeb

data

info.json

  • install_msg is missing from info.json this should explain that this cog comes with bundled data.

weeb.py

  • Lines 26, 34, 42: usage="[c]" does not make sense here. You’ve described c should be used after but in the usage it makes the help display Syntax: [p]uwu [c] which is not helpful. You should be descrbing your optional arguments better even the default option with docstrings explaining the only current option is c as meaning delete would make more sense here.

  • Line 50-58: re-defining error’s like this is unnecessary as it’s already handled by Red/discord.py. Furthermore this breaks i18n which would have already been handled by Red/discord.py when sending the error.

Unfortunately because I found code taken without attribution I am rejecting this application and you may not re-apply for a minimum of 1 month from today and not until all the non-optional notes have been addressed.

#4
#5