Uh oh!
There was an error while loading. Please reload this page.
add a pr template - #4334
Conversation
KendallDoesCoding
commented
Feb 5, 2022
Please review and get back to me on this. |
ghost
commented
Jul 30, 2022
The template content seems fine to me, however I just wonder whether we really need it or not? Because Nodejs.org isn't the same as Nodejs application. Waiting for others' suggestions. |
ovflowd
commented
Aug 3, 2022
I kinda agree here in the standpoint of this node being the main A lot of people ignore the |
I'm proceeding with the merge here 🙂 (Once the author rebases the branch) |
WaleedAshraf
left a comment
There was a problem hiding this comment.
In my personal experience, I have seen that people start to ignore such checks or it adds more friction for the first-time contributor. Also, this is the only repo where we have such a PR template.
Maybe we can copy the general PR guidelines in this repo from here:
https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/doc/contributing/pull-requests.md
Also, there is no issue with this specific request. Would it be better to add this after some discussion with contributors?
I'm not against it. I just felt that better to add such changes if there has been enough ask for it.
Regular contributors might be able to address it better than I can.
| - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) | ||
| - [ ] This change requires a documentation update | ||
| # Proposed Changes |
There was a problem hiding this comment.
Please add a newline to separate headers, and use the correct heading level (only using one H1 per document).
| # Proposed Changes | |
| ##Proposed Changes | |
| - change 1 | ||
| - change 2 | ||
| # Additional Info |
There was a problem hiding this comment.
Please add a newline to separate headers, and use the correct heading level (only using one H1 per document).
| # Additional Info | |
| ##Additional Info | |
Uh oh!
There was an error while loading. Please reload this page.
| ## Type of change | ||
| Please delete options that are not relevant. |
There was a problem hiding this comment.
This should probably be a comment:
| Please delete options that are not relevant. | |
| <!--Please delete options that are not relevant.--> |
| - [ ] Any dependent changes have been merged and published in downstream modules | ||
| - [ ] I have checked my code and corrected any misspellings | ||
| # Screenshots |
There was a problem hiding this comment.
Please use correct headers:
| # Screenshots | |
| ##Screenshots |
bnb
commented
Oct 10, 2022
commented a few changes should this get merged. |
bnb
commented
Oct 18, 2022
@aymen94 why'd you merge without addressing requested changes? |
ovflowd
commented
Oct 18, 2022
This shouldn’t have been merged. @aymen94 could you please revert? |
Updated PR from #4299