[APPROVED] Kreusadacogs

Discord Name: Kreusada#6666
GitHub Repository (Must be V3): https://github.com/kreus7/kreusadacogs
Description: Cogs for Red-DiscordBot. Including utility, as well as fun and games for your guild.

Howdy! Since I’m dropping this link in the other apps, I’m going to also post it here so you can see it too (even though your app was made after these docs were finalized). Double check that your repo meets these requirements before it is officially reviewed to ensure the application process is as smooth as possible!

Sure thing, I’ve had another look.

Hi Kreusada, thank you for your patience while I review your repo.

Commit hash at time of review - 3f03bb7e1308114fa27d97f218b21ad476d1aa9d

advanceduptime

info.json

  • install_msg Should explain that this replaces the core bots uptime command.

uptime.py

  • Line 29: You import timedelta but never use it. (Optional)

bubblewrap

info.json

  • You should have "min_bot_version" : "3.4.6", in here otherwise people with older versions of Red will break every time someone uses the only command in this cog.

dehoister

dehoister.py

  • Line 32: You import bold but never use it. (Optional)
  • Lines 160, 187, 268: You should check the bots permissions and hierarchy prior to making these calls to avoid unnecessary API calls. (Optional)
  • Line 322: This is mostly unnecessary since users can control whether or not to use the modlog via [p]modlogset cases having two ways to enable/disable is unintuitive for users when using cores modlog. (Optional)

pinginvoke

pingi.py

  • Line 25: You import discord but never use it. (Optional)
  • Line 83: You should use await self.config.botname.clear() that way you don’t store the null value in config indefinitely. (Optional)

pingoverride

info.json

  • install_msg should explain that this replaces cores default ping command with a new one.

sendcards

sendcards.py

  • Line 26: You import asyncio but never use it. (Optional)

staff

staff.py

  • Line 26: You import asyncio but never use it. (Optional)
  • Line 28: You import timedelta but never use it. (Optional)
  • Line 72: You should use await self.config.guild(ctx.guild).channel.clear() that way you don’t store the null value in config indefinitely. (Optional)
  • Line 85: You should use await self.config.guild(ctx.guild).role.clear() that way you don’t store the null value in config indefinitely. (Optional)
  • Line 154: I think you meant this to be conspicuous activity since inconspicuous means “not clearly visible or attracting attention; not conspicuous.”

General comments: Your usage of i18n in cogs you have included it is incomplete. You have the cog_i18n() function setup but that will only translate docstrings. All the response messages I have seen don’t have any signifier for redgettext to pull for translators to use. Your extensive use of f-strings in these cogs also have to be completely changed for responses to work with i18n. While i18n isn’t required and without any locale files these issues won’t appear, but if someone does want to create their own locale file they won’t be able to get all the correct strings.

1 Like

Hi Trusty, thanks a lot for your review - the requested changes have now been made. I completely removed the use of i18n as this was just something I wanted to experiment and never got round to actually working with.

Thanks for making those changes. I also wanted to clarify I wasn’t saying you should remove the i18n you had in there just explaining that parts are missing and the extensive use of f-strings could be problematic trying to make i18n fully compliant. Marking this as approved.