Skip to content
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

chore(template): improve PullRequest template #5622

Closed
wants to merge 8 commits into from

Conversation

ClxUne09
Copy link
Contributor

@ClxUne09 ClxUne09 commented Sep 15, 2024

Prerequisites

  • I have read and understood the contributing guide.
  • The commit message follows the conventional commits guidelines.
  • Tests for the changes have been added (for bug fixes / features).
  • Docs have been added/updated (for bug fixes / features).

Description

  • Added sections for clear description including summary, problem, solution, and impact.
  • Updated Helpful Resources section with links to contributing guides, Git practices, and community standards.

05e491b:

  1. Commitlint Rules: Added a reference to commitlint rules and standards to ensure that commit messages follow guidelines.
  2. Extended Description: Expanded the description section to include an additional point for indicating which issues the PR resolves or closes.

These updates aim to enhance clarity and ensure that pull requests adhere to our contribution and commit message standards.

- Added sections for clear description including summary, problem, solution, and impact.
- Updated Helpful Resources section with links to contributing guides, Git practices, and community standards.
ClxUne09 and others added 4 commits September 15, 2024 22:11
1. **Commitlint Rules:** Added a reference to commitlint rules and standards to ensure that commit messages follow guidelines.
2. **Extended Description:** Expanded the description section to include an additional point for indicating which issues the PR resolves or closes.

These updates aim to enhance clarity and ensure that pull requests adhere to our contribution and commit message standards.
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
[CONTRIBUTING.md]: https://github.com/JanDeDobbeleer/oh-my-posh/blob/main/CONTRIBUTING.md
---

<br>
Copy link
Owner

Choose a reason for hiding this comment

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

why are the linebreaks here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line breaks are used to emphasize the main PR description section, making it easier to focus on providing a clear and concise overview of the changes.

- **[Oh My Posh Git Guide][git-guide]:** For understanding Git practices and commands.
- **[Discussions Page][discussions]:** Join project discussions and ask questions.
- **[GitKraken Guide][gitkraken]:** Recommended tool for managing Git repositories.
<!-- - **[Code Structure Guide][code-structure]:** Understand the overall organization of our codebase. -->
Copy link
Owner

Choose a reason for hiding this comment

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

this isn't relevant here

Copy link
Contributor Author

@ClxUne09 ClxUne09 Sep 22, 2024

Choose a reason for hiding this comment

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

I think it's somewhat related to the pull request. It helps contributors better understand the PR.
I think adding this section to the docs is a good idea; we can link to the page we’ll create in the template. We can use this interim solution until the docs section is finalized.


---

### Helpful Resources
Copy link
Owner

Choose a reason for hiding this comment

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

there's a reason this was hidden, we shouldn't see this after creation

Copy link
Contributor Author

@ClxUne09 ClxUne09 Sep 22, 2024

Choose a reason for hiding this comment

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

Are you sure you want to comment that? It has a styling aspect for the preview, but it will also take up some space.

- **[Community Standard][community]:** Engage with the community and understand our community standards.
- **[Contributing Guide][contributing]:** Read and understand our guidelines for contributing.
- **[GitHub’s Pull Request Review Guide][gh-pr-review]:** Additional guidance on the review process.
<!-- - **[Pitfalls to Avoid][pitfalls]:** Common mistakes to watch out for when contributing. -->
Copy link
Owner

Choose a reason for hiding this comment

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

this isn't relevant here

Copy link
Contributor Author

@ClxUne09 ClxUne09 Sep 22, 2024

Choose a reason for hiding this comment

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

I think it's somewhat related to the pull request. It helps contributors better understand the PR.
I think adding this section to the docs is a good idea; we can link to the page we’ll create in the template. We can use this interim solution until the docs section is finalized.

@ClxUne09
Copy link
Contributor Author

Did I make a mistake that I should fix?

@JanDeDobbeleer
Copy link
Owner

JanDeDobbeleer commented Sep 30, 2024

Did I make a mistake that I should fix?

@ClxUne09 I was in doubt if I should merge this. I wasn't immediately convinced by the guidelines as I'm not really missing anything usually. When I get that feeling I let things marinate a bit, but TBH, I still feel the same.

@ClxUne09
Copy link
Contributor Author

Did I make a mistake that I should fix?

@ClxUne09 I was in doubt if I should merge this. I wasn't immediately convinced by the guidelines as I'm not really missing anything usually. When I get that feeling I let things marinate a bit, but TBH, I still feel the same.

You’re right; these changes focus more on the details rather than the actual parts. I will close this pull request for now. Thank you for your reply.

@ClxUne09 ClxUne09 closed this Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants