-
-
Notifications
You must be signed in to change notification settings - Fork 40
Add role and emote info commands #444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's Guide by SourceryThis pull request adds two new subcommands to the existing 'info' command: 'roles' and 'emotes'. These subcommands allow users to list all roles and emotes in the server, respectively. The changes are implemented by adding two new asynchronous methods to the Info cog class. File-Level Changes
Tips
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Atmois - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @info.command(name="roles", description="Lists all roles in the server.") | ||
| async def roles(self, interaction: discord.Interaction) -> None: | ||
| """ | ||
| List all roles in the server. | ||
| Parameters | ||
| ---------- | ||
| interaction : discord.Interaction | ||
| The discord interaction object. | ||
| """ | ||
| if not interaction.guild: | ||
| return | ||
|
|
||
| guild = interaction.guild | ||
| roles = [role.mention for role in guild.roles] | ||
|
|
||
| embed = EmbedCreator.create_info_embed( | ||
| title="Server Roles", | ||
| description=f"Role list for {guild.name}", | ||
| interaction=interaction, | ||
| ) | ||
|
|
||
| embed.add_field(name="Roles", value=", ".join(roles), inline=False) | ||
|
|
||
| await interaction.response.send_message(embed=embed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Refactor common code between 'roles' and 'emotes' commands
The 'roles' and 'emotes' commands have very similar structures. Consider refactoring the common parts into a separate method to reduce code duplication and improve maintainability.
async def _create_list_embed(self, interaction: discord.Interaction, title: str, items: list, item_name: str) -> None:
if not interaction.guild:
return
embed = EmbedCreator.create_info_embed(
title=title,
description=f"{item_name} list for {interaction.guild.name}",
interaction=interaction,
)
embed.add_field(name=item_name, value=", ".join(items), inline=False)
await interaction.response.send_message(embed=embed)
| if not interaction.guild: | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Improve error handling when there's no associated guild
Instead of silently returning when there's no guild associated with the interaction, consider providing feedback to the user, explaining why the command couldn't be executed.
| if not interaction.guild: | |
| return | |
| if not interaction.guild: | |
| await interaction.response.send_message("This command can only be used in a server.", ephemeral=True) | |
| return |
Add two subcommands to show role list and emote list
Closes #430
Closes #431