Skip to content

Conversation

@HikariNee
Copy link
Contributor

@HikariNee HikariNee commented Aug 25, 2024

Description

Renames some things for readability removes an inaccurate docstring.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Summary by Sourcery

Improve code readability by renaming variables and function parameters, and remove an inaccurate docstring.

Enhancements:

  • Rename variables and function parameters for improved readability.

Documentation:

  • Remove an inaccurate docstring to prevent misinformation.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Aug 25, 2024

Reviewer's Guide by Sourcery

This pull request focuses on improving code readability and accuracy by renaming variables, updating function parameters, and removing an inaccurate docstring. The changes are primarily in the tux/cogs/misc/run.py file, affecting regular expression variable names, function parameters, and return type annotations.

File-Level Changes

Change Details Files
Renamed regular expression variables for clarity
  • Changed 'ansi_escape' to 'ansi_re'
  • Changed 'remove_ticks' to 'ticks_re'
tux/cogs/misc/run.py
Updated function parameter names for better readability
  • Changed 'ticks' parameter to 'st' in the 'remove_backticks' function
tux/cogs/misc/run.py
Modified return type annotation in 'generalized_code_executor' function
  • Removed '
None' from the return type annotation, now it's just 'tuple[str, str, str]'
Updated regular expression usage in functions
  • Changed 'ansi_escape.sub()' to 'ansi_re.sub()' in 'remove_ansi' function
  • Changed 'remove_ticks.sub()' to 'ticks_re.sub()' in 'remove_backticks' function
tux/cogs/misc/run.py

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @HikariNeee - I've reviewed your changes - here's some feedback:

Overall Comments:

  • The return type annotation for generalized_code_executor has changed from tuple[str, str, str] | None to tuple[str, str, str]. Please confirm if this is intentional and that the method no longer returns None in any case.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@HikariNee
Copy link
Contributor Author

The change is intentional since it never returns None.

@kzndotsh kzndotsh merged commit 97485d3 into allthingslinux:main Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants