[APPROVED] Malarne - discord_cogs

#1

Discord Name:
Malarne#1418

GitHub Repository (Must be V3):

Description:
Miscellaneous cogs.

Now in english by default o/

Boobs: v3 update of the oboobs cog
League: League of legend cog, displaying infos like game players’elo, top masteries of an account, game history (slow, need an update) using the Neeko Python API (made by myself, unpublished now)
Leveler: A totally new Leveler for v3. Customizable [p]profile (background, note), and a lot of features !
Note: you can put the cog in french ! Use [p]set locale fr-FR
Note2: by default, whitelist is enabled, check out [p]levelerset channel whitelist to toggle it on/off :slight_smile:

assigned Redjumpman #2
#3

Thank you for resubmitting. I have conducted a preliminary review of your application, with a more thorough review to follow. These are a few minor things I thought you could work on, until I finish my final review. Let me know if you have any questions.

Main Level

  • Info.json author key contains an incorrect value, should be a list of strings.
  • all keys should be lower case in your info.json file.

Boobs

  • Missing an info.json file
  • Make sure to include in that info.json that this cog is NSFW
  • Remove instances of pass_context=True, this is default now.
  • You have a lot of variation in the naming scheme here: Boobs (folder), oboobs (filename), OboobsC(class name).
    Those need to all be the same name, (class will be CamelCase) to provide consistency in reloading and calling help.

League

  • class League needs a docstring
  • game command is missing a docstring
  • author key in your info.json file contains an incorrect value. It should be a list of strings.

Leveler

  • info.json file has an incorrect value for author. It should be a list of strings.
  • Main class, Leveler is missing a doc string.
  • blacklist toggle command is missing a docstring
#4

I just made all those little changes to the related cogs
Thanks for your time :slight_smile:

#5

Sorry for the wait, here is my final review:

League Cog

  • In your neeko module (why was this called neeko?) You are generating multiple sessions per request. I suggest looking at the aiohttp docs for more information on this issue. The code will look something like this:
def __init__(self, *args, **kwargs): 
        self._session = aiohttp.ClientSession()

# Closes the session when the cog is unloaded.
def __unload(self):
    asyncio.get_event_loop().create_task(self._session.close())
  • You need to provide the user a better way of setting an API key for this cog. requiring the user to edit the cog to hard-code their API is very inconvenient. I suggest creating a command to set the API key.

oboobs

Same thing here as with the League cog. You should be reusing your session instead of creating one every time you make a request.

Leveler

  • Your font should be in a data folder, which is accessed through Red’s bundled_data. The statement from . import __path__ will then be unnecessary.

  • It looks like you need to really lint this cog. You aren’t even using the following imports:
    mod, cog_data_path, namedtuple, urllib, randint

  • On line 319, The following line: if not grade in message.author.roles: should be rewritten as
    if grade not in message.author.roles: otherwise there is a bug.

  • You should also probably be reusing the session here as well for get_avatar, since these should all be pointing to a discord link, but you can leave backgrounds as is, since the link could be pointing anywhere.

Let me know when you have fixed these issues and I’ll test it out. Assuming I don’t run into a bugs from running the commands after these changes, you will be approved!

#6

If i didn’t forget anything, everything should be fixed.
Just a little point for League cog, as long as it is now broken due to recent api changes, i still made the changes and checked if it was working using some debugs, the api key works fine, i just don’t really know for requests, but it should be okay, and if there’s something wrong i’ll just fix it when i’ll take the time to update it (which should be really soon ^^ )

#7

It appears that the data folder for the Leveler cog is missing from your repo.

#8

fixed that, thanks ^^

#9

After looking through your commit history and reviewing your updates, I am approving this application. Congratulations! I will be locking this thread from this point on. If you have any questions please feel free to contact me on discord, or send a message to the QA group on CogBoard. Please note that it may take up to 24 hours for all cog creator benefits to be visible.

closed #10