-
-
Notifications
You must be signed in to change notification settings - Fork 40
docs(globally): Add docstrings and refactor various code #194
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
… functions for datetime conversion feat(functions.py): add compare_guild_channel_changes and compare_member_changes functions for comparing changes in GuildChannel and Member instances respectively
…ganize the file for better readability refactor(pyproject.toml): reorder dependencies and add new ones for better project management feat(tux/utils/constants.py): add new constants for gate logging and tux logs to improve logging capabilities
…rnary expressions for cleaner code and improved readability
This commit introduces a new file, audit.py, which contains a class for audit logging. This class includes listeners for various events such as channel creation/deletion/update, guild update, emoji and sticker updates, integration creation/update/deletion, role creation/deletion/update, scheduled event creation/deletion/update, stage instance creation/deletion/update, and thread creation/deletion/update. Each listener sends an embed message to the audit log channel detailing the event. feat(logging): add new logging cog 'gate.py' for gate logging refactor(logging): update 'member.py' to use DatabaseController and simplify member update logging feat(logging): add new logging cog 'message.py' for guild logging refactor(logging): update 'mod.py' to improve ban/unban logging and remove duplicate code
…ry condition checks refactor(users.py): rename variable 'existing_user' to 'user' for better readability fix(users.py): ensure update_user method updates all fields regardless of changes feat(users.py): import UsersUpdateInput from prisma.types to use as data type for update_user method
feat(embeds.py): add default return values for avatar and username when user and interaction are None to prevent null values
…sistency feat(cog_loader.py): add exception handling to provide more robust error handling style(main.py): add comments to improve code readability refactor(console.py): update comments for better understanding of the code feat(functions.py): add convert_to_seconds function to convert formatted time string to seconds docs(functions.py): update docstrings to numpy style for better readability and consistency refactor(functions.py): improve is_integer and is_float functions by using is_convertible_to_type function for DRY code
refactor(error_handler.py): update error handling logic to improve error messages and logging fix(error_handler.py): handle app command errors separately to avoid conflicts style(error_handler.py): improve code readability and maintainability by simplifying error handling logic feat(test_error_handler.py): add new file for testing error handling in discord.py This new file contains a Cog class with commands to raise every type of discord.py error for testing purposes. feat: add error handling commands to ErrorTests cog in discord bot This commit adds a series of commands to the ErrorTests cog in the discord bot. Each command raises a specific exception when invoked, allowing for testing and debugging of error handling mechanisms. The exceptions covered include DisabledCommand, CommandInvokeError, CommandOnCooldown, MaxConcurrencyReached, various ExtensionErrors, ClientException, and CommandRegistrationError. refactor(error_handler.py): rename ErrorHandler to UnifiedErrorHandler for clarity feat(error_handler.py): add support for traditional command errors and app command errors fix(error_handler.py): improve error message for better user experience style(error_handler.py): refactor code for better readability and maintainability
…ring and guild check refactor(admin/load.py): rename 'cog' to 'ext', add detailed docstring and error handling refactor(admin/reload.py): rename 'cog' to 'ext', add detailed docstring and error handling refactor(admin/sync.py): add detailed docstring, guild check and change ctx.send to ctx.reply refactor(admin/unload.py): rename 'cog' to 'ext', add detailed docstring and error handling The refactor was done to improve the readability and maintainability of the code. The renaming of 'cog' to 'ext' was done to better reflect the nature of the variable. The addition of detailed docstrings provides a clear understanding of the purpose and usage of the commands. The enhanced error handling provides more specific feedback to the user. The guild check ensures that the commands are only used in a server context.
… for better code understanding refactor(purge.py): remove unused imports and methods, use EmbedCreator for creating embeds refactor(report.py): rename ConfirmModal to ReportModal for better semantics feat(purge.py): add error handling for non-text channels and invalid number of messages feat(report.py): add docstring to report method, improve report submission process docs(slowmode.py, timeout.py, unban.py): add docstrings to methods for better code readability and understanding refactor(slowmode.py): change default slowmode delay from 5 to 4 seconds for better user experience style(slowmode.py, timeout.py, unban.py): add line breaks for better code readability fix(unban.py): remove redundant logging of unban command usage, logging is now handled after successful unbanning docs(warn.py): add docstrings to methods for better code readability and understanding refactor(warn.py): remove error handling from create_infraction method to improve separation of concerns
…usability docs(archwiki.py): add docstrings to query_archwiki and archwiki methods for better code understanding style(archwiki.py): improve code readability by using self.base_url instead of base_url docs(avatar.py): add docstrings to avatar method for better code understanding refactor(avatar.py): improve code readability by assigning avatar url to a variable feat(guide.py): add meta_fields, support_fields, resources_fields for better code organization docs(guide.py): add docstrings to guide method for better code understanding refactor(guide.py): improve code readability by using fields variables in embed.add_field feat(info.py): add logger to log command usage docs(info.py): add docstrings to server, tux, member, irc methods for better code understanding refactor(info.py): improve code readability by reordering code blocks and assigning values to variables refactor(membercount.py, ping.py, poll.py, remindme.py): add docstrings for better code readability feat(membercount.py, ping.py, poll.py, remindme.py): add logging for user command usage for better tracking fix(remindme.py): handle exceptions when getting and creating reminders to prevent app crash refactor(remindme.py): move convert_to_seconds function to utils/functions.py for better code organization style(remindme.py): improve code comments for better understanding docs(rolecount.py, tldr.py): add docstrings to methods for better code understanding refactor(rolecount.py, tldr.py): improve variable typing for better type checking refactor(tldr.py): modify subprocess handling for better error management and readability style(tldr.py): improve error messages for user clarity
… understanding refactor(database/controllers): change docstring format for better readability and consistency feat(database/controllers): add channel_id and guild_id parameters to create_reminder method in RemindersController to associate reminders with specific channels and guilds docs(roles.py, snippets.py): update docstrings to numpy style for better readability and consistency refactor(roles.py): modify default values in update_role method to handle None values correctly feat(roles.py): add docstrings to sync_role and delete_role methods for better understanding of their functionality feat(snippets.py): add docstrings to __init__, get_all_snippets, get_all_snippets_sorted, get_snippet_by_name, create_snippet, delete_snippet, and update_snippet methods for better understanding of their functionality docs(database/controllers): update docstrings in user_roles.py and users.py for better clarity and consistency refactor(users.py): optimize sync_user method to only update fields that have changed to improve performance
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 @kzndotsh - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟡 Security: 1 issue found
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| return 0 if current_value != 0 else total_seconds | ||
|
|
||
|
|
||
| def datetime_to_unix(dt: datetime | None): |
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 (code_refinement): Consider using a more descriptive function name.
The function name datetime_to_unix could be more descriptive. Consider renaming it to datetime_to_discord_timestamp to reflect that it returns a Discord timestamp string, not just a Unix timestamp.
| def datetime_to_unix(dt: datetime | None): | |
| def datetime_to_discord_timestamp(dt: datetime | None): |
|
|
||
| return | ||
|
|
||
| # Create the description for the poll 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.
nitpick (code_refinement): Redundant comment for straightforward code
| The member to get the avatar of. | ||
| """ | ||
|
|
||
| avatar = member.avatar.url if member.avatar else "Member has no avatar." |
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 (security): Potential privacy concern with direct avatar URL access
Directly accessing a user's avatar URL without checks might lead to privacy issues or data leaks if the avatar is sensitive. Consider adding additional checks or permissions.
|
|
||
| else: | ||
| missing = "Role" if role is None else "Emoji" | ||
| missing: Literal["Role", "Emoji"] = "Role" if role is None else "Emoji" |
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 (code_refinement): Use of Literal type may be unnecessary
The use of a Literal type for the 'missing' variable might be over-specifying the type. Consider simplifying to a basic string since it does not interact with other parts of the system that would require such strict typing.
| missing: Literal["Role", "Emoji"] = "Role" if role is None else "Emoji" | |
| missing = "Role" if role is None else "Emoji" |
| def convert_to_seconds(time_str: str) -> int: | ||
| """ | ||
| Converts a formatted time string with 'd' for days, 'h' for hours, and 'm' for minutes into total seconds. | ||
| Any unexpected format leads to returning 0. | ||
| Parameters: | ||
| ---------- | ||
| time_str : str | ||
| The formatted time string to convert to total seconds. | ||
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 (testing): Missing test cases for convert_to_seconds function.
The function convert_to_seconds is crucial for time conversion and should have associated unit tests to verify that all branches and edge cases are handled correctly, such as input with unexpected characters, no units, or multiple units.
| def datetime_to_elapsed_time(dt: datetime | None): | ||
| """ | ||
| Takes a datetime and computes the elapsed time from then to now in the format: X years, Y months, Z days. | ||
| Parameters: | ||
| ---------- | ||
| dt : datetime | ||
| The datetime object to compute the elapsed time from. | ||
| Returns: |
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 (testing): Missing test cases for datetime_to_elapsed_time function.
This function calculates elapsed time and should be tested for various time differences, including edge cases like the same day, leap years, and transitions between months or years.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
No description provided.