-
Notifications
You must be signed in to change notification settings - Fork 0
[Feature] Kaylee's Cog: Poll #6
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideAdds a new onboarding cog “KayleeCog” that implements a /poll slash command with three choices, builds an emoji-prefixed embed, logs the poll, sends the embed, and adds emoji reactions for voting. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @kaylee-xie - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 4 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 and I'll use the feedback to improve your reviews.
| #change everything with "Ping" | ||
| class KayleeCog(commands.Cog): |
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: Remove or clarify the leftover comment
The #change everything with "Ping" comment appears to be an outdated TODO. Please remove it or update it to reflect the current intent.
| #change everything with "Ping" | |
| class KayleeCog(commands.Cog): | |
| class KayleeCog(commands.Cog): |
| embed = discord.Embed( | ||
| title = "Poll: " + question, | ||
| description = message, | ||
| #color = colors.POLL, #couldn't get it to work :( gave me errors |
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: Remove or properly re-enable the commented-out color
Uncomment and fix if colors.POLL is correct; otherwise, remove the commented line to avoid dead code.
Suggested implementation:
embed = discord.Embed(
title = "Poll: " + question,
description = message,
color = colors.POLL,
)If colors.POLL is not defined or imported, you will need to:
- Ensure
colorsis imported andPOLLis defined as a valid Discord color (e.g.,discord.Color.blue()or an integer). - If you do not want to use a color, simply remove the commented line entirely.
| description = message, | ||
| #color = colors.POLL, #couldn't get it to work :( gave me errors | ||
| ) | ||
| self.logger.info(message) #logs message |
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: Include more context in the log entry
Logging only the options string may not provide enough information for debugging. Log the question and choices for better context.
Suggested implementation:
self.logger.info(
"Poll created | Question: %r | Choices: %r | Message: %r",
question,
choices if 'choices' in locals() else 'N/A',
message
) #logs messageIf the variable choices is not defined in this scope, replace it with the actual variable or data structure that contains the poll choices/options. Adjust the logging format as needed to match your codebase's conventions.
|
|
||
| @app_commands.guilds(discord.Object(id=settings.DEBUG_GUILD_ID)) | ||
| @app_commands.command(name="poll", description="Custom poll with 3 choices") | ||
| async def poll(self, interaction: discord.Interaction, question: str, choice_1: str, choice_2: str, choice_3: str): |
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.
question: Consider adding input validation for the poll fields
Currently, empty or duplicate choices can be submitted. Please ensure question and all choices are non-empty, within length limits, and unique before creating the embed.
| descrip = emojis[i] + " " + choices[i] | ||
| descriptions.append(descrip) | ||
| message = "\n".join(descriptions) #combine all into one string | ||
|
|
||
|
|
||
| #description embed | ||
| embed = discord.Embed( | ||
| title = "Poll: " + question, | ||
| description = message, | ||
| #color = colors.POLL, #couldn't get it to work :( gave me errors | ||
| ) |
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.
issue (code-quality): Use f-string instead of string concatenation [×3] (use-fstring-for-concatenation)
For the 'poll' cog
Summary by Sourcery
Add Kaylee onboarding cog providing a slash command to create custom 3-choice polls with embed messages and reaction-based voting
New Features: