github: add issue and pull request templates #5291
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| ### Issue details | ||
|
|
||
| _Please provide issue details here_. | ||
|
|
||
| ### Steps to reproduce/test case | ||
|
|
||
| _Please provide necessary steps for reproduction of this issue, or better the | ||
| reduced test case (without any external dependencies)_. | ||
|
There was a problem hiding this comment. "Without any external dependencies, if possible" might be friendlier. |
||
|
|
||
| ### Affected node.js versions | ||
|
|
||
|
There was a problem hiding this comment. Perhaps add a statement asking the reporter to fill in the specific minor and patch level also There was a problem hiding this comment. Just ask for the version, no need for the multiple choice. |
||
| - [ ] master | ||
| - [ ] v5.x | ||
| - [ ] v4.x | ||
| - [ ] v0.12 | ||
| - [ ] v0.10 | ||
|
|
||
| ### Affected platforms | ||
|
There was a problem hiding this comment. I think asking for |
||
|
|
||
| - [ ] linux | ||
|
There was a problem hiding this comment. Nit: For this and the others, we probably want to style as the project/vendor/whoever styles it. So, |
||
| - [ ] windows | ||
| - [ ] OS X | ||
| - [ ] freebsd | ||
| - [ ] solaris | ||
| - [ ] other _(please specify which)_ | ||
|
|
||
| ### Core subsystem (if known) | ||
|
|
||
| - [ ] buffer | ||
| - [ ] child_process | ||
| - [ ] cluster | ||
| - [ ] crypto | ||
| - [ ] dgram | ||
| - [ ] dns | ||
|
|
||
| - [ ] doc | ||
| - [ ] fs | ||
| - [ ] http | ||
| - [ ] https | ||
| - [ ] net | ||
| - [ ] tls | ||
| - [ ] tty | ||
| - [ ] vm | ||
| - [ ] zlib | ||
| - [ ] other _(please specify which)_ | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. I don't think anyone will know this stuff. If we have the section at all, don't say sub-system, say "module", and expect the name from the docs (fs, http, net, etc.), with maybe "docs/website" as an additional. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| ### Description of change | ||
|
|
||
| _Please provide description of change here_. | ||
|
There was a problem hiding this comment. Period within italics? |
||
|
|
||
| ### Pull Request check-list | ||
|
|
||
| _Please make sure to review and check all of these items_: | ||
|
|
||
| - [ ] Does `make test` pass after this change? | ||
|
There was a problem hiding this comment. "Does Or something mentioning windows platform testing |
||
| - [ ] Is commit message formatted according to [CONTRIBUTING.md][0] | ||
|
There was a problem hiding this comment.
|
||
|
|
||
|
There was a problem hiding this comment. Test or benchmark included? There was a problem hiding this comment. Should "+1 review " be a step here? ( I'm +1 about using these checkboxes for PRs, just to be clear.) There was a problem hiding this comment. What do you mean by "+1 review"? There was a problem hiding this comment. Maybe add something along the lines of: "Note: any of the above can be amended after and is not required to open a PR" There was a problem hiding this comment. They are quite required :) We won't land PR if any one of these is not present. There was a problem hiding this comment. Required to land, but not to open it. basically I'd like to note that there can be done while the PR is open, i.e. you can write docs after we think a change is good. There was a problem hiding this comment. What about saying above that these items could be checked after the PR is open, and if some of them are not done yet - they could be checked upon completion? There was a problem hiding this comment. @Fishrock123 does this wording sound better? There was a problem hiding this comment. I don't think so. I meant having a note at the bottom of the section stating these things are not required to open a PR and can be done afterwards / while the PR is open. |
||
| ### Affected core subsystem(s) | ||
|
|
||
| - [ ] buffer | ||
| - [ ] child_process | ||
| - [ ] cluster | ||
| - [ ] crypto | ||
| - [ ] dgram | ||
| - [ ] dns | ||
|
There was a problem hiding this comment. I'm not sure about the usability of this. After the issue is created this will turn into a list of checkboxes that is visible -- and anybody with write access can check them. Is that what we want? There was a problem hiding this comment. @mikeal we want to be able to tell subsystem without looking at the code. Do you have any alternative ideas on how it could be accomplished? There was a problem hiding this comment. Caine should be able to do some pretty simple analysis of the files that are in the PR to determine the sub-system. This seems like the easier thing to automate that we've talked about. There was a problem hiding this comment. Right now we don't have caine, and it should be beneficial to experiment with templates to see what works and what does not. There was a problem hiding this comment. Actually, won't the checkboxes turn into a list of tasks ? ( I'd suggest something like this: There was a problem hiding this comment. The checkboxes have the benefit of easily showing whats available as options. A comma delimited list one liner would lose that effect... The tasks list for every issue though would get unreal over time. There was a problem hiding this comment. Agree, will replace it. Ack. |
||
| - [ ] doc | ||
| - [ ] fs | ||
| - [ ] http | ||
| - [ ] https | ||
| - [ ] net | ||
| - [ ] tls | ||
| - [ ] tty | ||
| - [ ] vm | ||
| - [ ] zlib | ||
| - [ ] other _(please specify which)_ | ||
|
|
||
| ### SemVer | ||
|
|
||
| What [semver][1] change does this change require? | ||
|
|
||
|
There was a problem hiding this comment. Add |
||
| - [ ] patch _(no new APIs, no breaking changes)_ | ||
| - [ ] minor _(new APIs, no breaking changes)_ | ||
| - [ ] major _(breaking changes, or just too dangerous to be minor/patch)_ | ||
|
|
||
|
There was a problem hiding this comment. Relevant version would be good also for tracking LTS relevant changes. There was a problem hiding this comment. I'm actually going to remove this, because it is up to CTC to decide what it is. There was a problem hiding this comment. What if the submitter has a strong case for minor vs major? |
||
| ### LTS | ||
|
|
||
| Should this patch be backported to: | ||
|
There was a problem hiding this comment. probably unnecessary, if someone doesn't know to specify this they probably won't be tuned into the process enough anyways and we'll probably have to decide, I think? There was a problem hiding this comment. This sounds reasonable. I will change it once we will decide on general form of these files :) |
||
|
|
||
| - [ ] current LTS | ||
| - [ ] older versions? _(please specify the version numbers)_ | ||
|
|
||
| [0]: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md#step-3-commit | ||
| [1]: http://semver.org/ | ||
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.
Perhaps start with a friendly intro?
Thanks for wanting to report an issue you've found in node.js. Please fill in below template. If unsure about something, just do as best as you're able.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.
Also state that this is optional and not required
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.
Should there be a
### headerat the beginning of the issue? Will it be ugly when the issues are searched or aggregated? Maybe the first section can be header-less?