[APPROVED] CogsByAdrian

#1

Discord Name:
ThinkAdrian#1186

GitHub Repository:

Description:
One useful, and three fun cogs:

Crier: have the bot say or 🇸 🇭 🇴 🇺 🇹 messages into other channels,
Insult: insult a user
Killer: do nasty things to other users like Slap or Shoot
Penis: measure the penis of a user with 100% accuracy - conversion from the original V2 cog by TwentySix

TBA: Inventory – something fun (and not so small at all) using Economy
TBA: Urbanbot - based on my Slackbot, Urban will randomly participate in discussion with nasty comments
TBA: ThatsWhatSheSaid - randomly announces this terrible joke relevant to conversation!

assigned Sinbad #2
#3

commit hash at time of review: 3713a89f96b7b0a1d7ffa2daf163e1fbe3c9d31d

crier/crier.py:

L8: I don’t have a problem with the more lax base permissions with this recommendation, but it should likely be in the install message as well.

L17, L32: should be #channelName instead of @channelName in the docstring

L20: Should use ctx.bot.send_filtered for passing user input.

  • potential exception if you confirm the user would normally be able to mention everyone there, and confirm that as the intent with the user.

insult/insult.py
L1, L29: This should be done using aiohttp instead of requests

  • This is already a dependency, so you don’t need to worry about adding to your requirements, but it will prevent this from blocking other bot actions.

L43: looks like you forgot to send the formatted insult around this line.

  • cog does not work as a result

killer/killer.py:
L105:

killer['formatted', target['formatted']]

should be :

killer['formatted'], target['formatted']

killer/info.json: I think a few of the commands here warrant warning the user in install message of potentially objectionable content. There’s no reason to get rid of the commands, but users installing this shouldn’t end up surprised by it as a result.

penis/penis.py:

User’s of the original cog will find their size has changed. You can go along with that, but given you are calling it a successor to 26’s work, I’d suggest using str(user.id) instead of user.id for random seeding for consistency between v2 and v3 (Optional fix, recommendation only).

L34: remove the semicolon.

General formatting: 2 spaces for an indent technically works, but convention with python is 4 spaces or use tabs. Recommend changing this for the file, but it’s not a requirement.

If you have any questions about the requested changes, feel free to ask.

#4

It doesn’t seem that I had email replies activated, or I simply missed the emails.

Thank you for your feedback! I have understood all issues and shall fix these issues soon. Do I write a new reply when I’m done or is there some kind of tagging system for posts?

unassigned Sinbad #5
assigned TrustyJAID #6
#7

Hi @ThinkAdrian, I’ve been assigned to take over review of your application. Reply to this thread when you’ve completed Sinbad’s recommendations and I’ll look over it.

#8

Hello! Thank you for taking your time to review my repo. I have now addressed all issues and recommendations, because I have no objections!

#9

Awesome, just looked through and everything looks great! Marking this as approved.

1 Like
closed #10