Discord Name:
Sharky The King#0001
GitHub Repository (Must be V3):
Description:
A few cogs that I’ve had the chance to make after learning. Most of them are focus on moderation for servers. Charlimit, Lockdown, NewsPublish, Reports, Verify and Announcement are focused on server-management/moderation and Strawpoll is a fun cog to make polls using straw poll’s API.
2 Likes
Hi Sharky The King, thank you for your patience while I review your repo.
Commit hash at time of review - 7cb6c7e1eb4c1c2289e7f0431cca996c267dcb57
charlimit
charlimit.py
- Line 10: lol (Optional)
- line 124:
for shark
Nice (Optional) - Lines 180, 182, 241: log.info can get a bit spammy for users log files and log.error and log.exception also exist if you want people to really know what the issues are and log.debug is useful for testing the code for things while not spamming end user logs. (Optional)
lockdown
core.py
- Lines 49-52: Cross-server actions are not recommended, these are commented out but I still want to mention that activating a lockdown from some other server is really not a good idea. Especially not in the current state where the only check for permissions is mod and
manage_messages
. - Line 98: You’re allowing only people with
manage_channels
the ability to lockdown a server but allowing people with onlymanage_messages
to remove the lockdown, is this intentional? - Line 188: You might consider utilizing the the context manager better for example:
status = "Error"
async with self.config.guild(ctx.guild).channels() as chan:
if channel.id not in chan:
chan.append(channel.id)
status = "Added"
else:
chan.remove(channel.id)
status = "Removed"
await ctx.send(
f"New Status for {channel} set! - {status}".format(channel=channel.mention, status=status)
)
(Optional)
namegen
core.py
- Line 35: Typically it’s best practice to put the link to where you got stuff like this from including sites like stacktrace and stackoverflow.
reports
core.py
- Line 2: You import Union but it is never used. (Optional)
- Line 207-213: You might try using Red’s
start_adding_reactions
function to handle adding the reactions to a message instead. (Optional)
sharkytools
core.py
- Line 29 and 119: You should attempt to get the user object from the bots cache first before making the API call.
ctx.bot.get_user
will look in the bots cache first for a user object and then you can use thefetch_user
object.
verify
verification.py
- Lines 102-116: You need to account for hierarchy in this command for the role being supplied. In its current state a mod could potentially setup a role higher than their own in the hierarchy and leave the server and rejoin granting them more permissions than they had previously.
All the requested stuff have been changed…
Charlimit
- Line 10: Is kept because I loved that from when Kowlin had it
- 124: Changed to be consistent for channel
- Line 180, 182 Both changed to debug, 241 Is kept because imo I like it kept for when cog/bot is restarted.
Lockdown
This has been completely redone, there is no cross-server actions anymore due to unsafe usages, since discord has been a lot more stable lately, it’s not needed or wanted. I’d ask for you to do a complete redo review on this as the code is completely different than before.
Namegen
Added URL from what I could find, the code was from a friend so I’m not 100% sure it’s the same but it gives the same explanation from what I could find.
Reports
- Line 2: Removed union import
- Line 207-213: Didn’t use, interesting function and I love it but I wanted to have my version to allow for exception and log handling.
SharkyTools
- Line 29 and 119: I hope I resolved this, I attempted ctx.bot.get_user and if that failed it would attempt to fetch and then return False if it’s not a user.
Verify
- Line 102-116: handled for both bot’s hierarchy and user hierarchy.
Thank you for making those changes. Marking this as approved!