Skip to content

Conversation

@electron271
Copy link
Member

@electron271 electron271 commented Aug 28, 2024

Description

change stuff that isnt actually in tux and clarify stuff

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

How Has This Been Tested?

not applicable

Screenshots (if applicable)

Please add screenshots to help explain your changes.

Additional Information

Please add any other information that is important to this PR.

Summary by Sourcery

Revise the pull request template to improve clarity on how to document issue fixes and testing procedures, and update the checklist to reflect current project standards.

Documentation:

  • Update the pull request template to clarify instructions for including issue fixes and testing descriptions.

change stuff that isnt actually in tux and clarify stuff
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Aug 28, 2024

Reviewer's Guide by Sourcery

This pull request updates the PULL_REQUEST_TEMPLATE.md file to streamline and clarify the pull request process. The changes focus on simplifying the template, adding more specific instructions, and adjusting the checklist items to better reflect the project's workflow.

File-Level Changes

Change Details Files
Simplified and clarified the Description section
  • Removed the 'Pull Request Template' title
  • Consolidated the description instructions into a single paragraph
  • Added instruction to include 'Fixes #XX' for issue-related changes
.github/PULL_REQUEST_TEMPLATE.md
Updated the Type of Change section
  • Added an 'Other' option with a write-in field
.github/PULL_REQUEST_TEMPLATE.md
Revised the Checklist section
  • Added specification for code formatting with Ruff
  • Modified documentation change item to be conditional ('if needed')
  • Removed specific test-related items and replaced with a general testing confirmation
  • Removed the item about dependent changes
.github/PULL_REQUEST_TEMPLATE.md
Simplified the How Has This Been Tested section
  • Removed specific test examples
  • Added request for more specific testing information, including commands, arguments, and configuration details
.github/PULL_REQUEST_TEMPLATE.md

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 @electron271 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider encouraging more detailed testing information for significant changes or new features, while keeping it simple for minor updates.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟡 Documentation: 1 issue found

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.


- [ ] Test A
- [ ] Test B
Please describe how you tested your code. e.g describe what commands you ran, what arguments, and any config stuff (if applicable)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (documentation): Consider providing more structure for test description

While the open-ended format allows flexibility, you might want to include some prompts or examples to ensure comprehensive test descriptions.

Suggested change
Please describe how you tested your code. e.g describe what commands you ran, what arguments, and any config stuff (if applicable)
## Testing
Please describe how you tested your code:
- [ ] Commands run:
- [ ] Arguments used:
- [ ] Configuration details:
- [ ] Test cases covered:
- [ ] Edge cases considered:
- [ ] Any additional testing notes:

make guideline checking less time consuming
@kzndotsh kzndotsh merged commit 9427591 into main Aug 28, 2024
@kzndotsh kzndotsh deleted the electron271-patch-1 branch August 28, 2024 17:42
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.

4 participants