New PULL_REQUEST_TEMPLATE.md file #1637 #1639
Conversation
| <!-- What types of changes does your code introduce? --> | ||
| <!-- 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) --> |
There was a problem hiding this comment.
The above Types of Changes verbiage should match that of the existing CONTRIBUTING.md, add, try, update etc
There was a problem hiding this comment.
This section seems to be more for the actual naming of the branch. How would you suggest this is described within the changes section?
| ## Checklist: | ||
| - [ ] My code is tested. | ||
| - [ ] My code follows the WordPress code style. | ||
| - [ ] My code follows has proper inline documentation. |
There was a problem hiding this comment.
A link to the TESTING section of CONTRIBUTING.md should be included in this section
There was a problem hiding this comment.
Same questions here. Do we really want links within the PR template? I haven't seen any other templates include links like this before. If it's not in an HTML comment you're going to see a bunch of people not remove it and clutter up the actual content.
There was a problem hiding this comment.
I would say don't link as it could create the issue we already have. I know it's a good information source, but we consistently have an issue with people not clearing out this content.
| @@ -0,0 +1,20 @@ | |||
| ## Description | |||
There was a problem hiding this comment.
Probably above this should be a link CONTRIBUTING.md
There was a problem hiding this comment.
Do you suggest that it's linked within an html comment? I think if it's not then you'll see a bunch of issues with "Please read how to contribute prior to submitting an issue" or whatever verbiage prior to the actual issue. Could get annoying.
There was a problem hiding this comment.
I would -1 for linking for the reasons @DevinWalker has said.
|
|
||
| ## Screenshots (jpeg or gifs if applicable): | ||
|
|
||
| ## Types of changes |
There was a problem hiding this comment.
Is this really necessary? I'd rather use tags for most of these
There was a problem hiding this comment.
I've found screenshots to be very helpful, especially gifs, when reviewing a feature or UI fix/enhancement. Also, normal contributors don't have access to label our PRs.
There was a problem hiding this comment.
Screenshots are very helpful indeed. I was talking about the "Types of changes" but the labels permissions is a good point.
|
@mtias or @karmatosed, it would be nice to get it merged. Would you mind leaving your feedback here? I'm happy to help to get it into master. |
|
Thanks again @DevinWalker, sorry it took so long 👍 |
|
No problem. Happy to help.
On Thu, Sep 21, 2017 at 11:54 PM Stephen Edgar ***@***.***> wrote:
Thanks again @DevinWalker <https://github.com/devinwalker>, sorry it took
so long 👍
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1639 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABf7M0JLghtaygxEQIYXQNEd76vxfqLZks5sk1megaJpZM4OLcLj>
.
--
Devin Walker
Founder & Lead Developer
P:
858-449-6460 <javascript:void(0);>
W:
https://wordimpress.com
433 G Street #201
San Diego, CA 92101 <https://goo.gl/maps/BePgp>
<https://www.facebook.com/wordimpressive> <https://twitter.com/wordimpress>
<https://plus.google.com/u/0/b/117062083910623146392/+Wordimpress/about>
<http://wordimpress.com/>
|
PR Overview
Added a
PULL_REQUEST_TEMPLATE.mdfile as a starting point. This will encourage more descriptive PRs to improve ease of testing and review.If you have any suggested edits, just let me know!
Ref: #1637