-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Create a general pull_request_template.md #703
Conversation
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.
Mostly, these are back-end Markdown suggestions.
*List anything note-worthy here (potential issues, this needs to be merged to `master` before working, etc....).* | ||
*Don't forget to link any issues that this PR will solved.* | ||
|
||
**Please link any issue that this PR should solve if it's applicable** |
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.
**Please link any issue that this PR should solve if it's applicable** | |
<!-- Please link any issue that this PR solves if applicable --> |
*List anything note-worthy here (potential issues, this needs to be merged to `master` before working, etc....).* | ||
*Don't forget to link any issues that this PR will solved.* |
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.
*List anything note-worthy here (potential issues, this needs to be merged to `master` before working, etc....).* | |
*Don't forget to link any issues that this PR will solved.* | |
<!-- List anything note-worthy here (potential issues, this needs to be merged to `master` before working, etc....). | |
Don't forget to link any issues that this PR will solve. --> |
- [] PR name matches the format *New Feature: brief description of feature* | ||
|
||
|
||
## This PR adds/fixes... | ||
|
||
*List your features here and the benefits they bring.* |
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.
*List your features here and the benefits they bring.* | |
<!-- List your features here and the benefits they bring. --> |
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.
Why do we comment out this line though? Shouldn't the user see this?
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.
Why do we comment out this line, though? Shouldn't the user see this?
This works like in other programming languages: comments cannot be read by computers, but they can be read by the user. The purpose of the comments should only be read by the PR creator, not by other users. Hope I get to understand. 🙂
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.
I think that line should be there as an instruction to people opening a PR into the repo. If it's only readable to us, people might get confused.
Will this line shows up when someone open a PR into the repo? If not, I think we should stick with the old one
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.
Hmm, I think you misunderstood, sorry. 😅
When you use comments in Markdown, that's only readable when editing a comment or a Markdown file, but that can't be read when looking at the file/comment preview. Hope I get to understand now. 😅
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.
I see. In that case, I think we should stick with what we had. I want the people opening PRs to see that instruction and not have it hidden. Let me know if you had something else in mind for this. If not, I'll dismiss this in a few days
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.
Hmm, I think you misunderstood again... 😅
Try to use this code in a comment:
<!-- List your features here and the benefits they bring. -->
Before submitting it, click on the Preview
button, check the content, and then go back to the Write
button.
Co-authored-by: David Leal <halfpacho@gmail.com>
I just changed all the instructions to comments. I think this will work better |
Co-authored-by: David Leal <halfpacho@gmail.com>
Good idea Co-authored-by: David Leal <halfpacho@gmail.com>
Co-authored-by: David Leal <halfpacho@gmail.com>
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.
Looks good. Great work, @Thomas-Boi; thank you! 👍 🎉
I'll merge this. If there's something wrong or something you missed, you can create another PR to fix it. 🙂 |
* Create a general pull_request_template.md * Apply suggestions from code review Co-authored-by: David Leal <halfpacho@gmail.com> * Add line about base of PR in pr template * Update PR and Issue templates * Update .github/drafts/PULL_REQUEST_TEMPLATE/new_icon.md Co-authored-by: David Leal <halfpacho@gmail.com> * Update .github/drafts/PULL_REQUEST_TEMPLATE/new_icon.md Good idea Co-authored-by: David Leal <halfpacho@gmail.com> * Update .github/drafts/PULL_REQUEST_TEMPLATE/new_icon.md Co-authored-by: David Leal <halfpacho@gmail.com> * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: David Leal <halfpacho@gmail.com>
This fixes #699.
Details:
Note:
master