github: add issue and pull request templates #5291

indutny · 2016-02-17T20:48:43Z

See: https://github.com/blog/2111-issue-and-pull-request-templates

cc @nodejs/ctc

indutny · 2016-02-17T20:49:24Z

cc @bnoordhuis @piscisaureus @chrisdickinson @mikeal

indutny · 2016-02-17T20:50:01Z

This could be used by bot in future, meanwhile it should be simplify things for people who tag issues manually. cc @mscdex @thealphanerd btw

indutny · 2016-02-17T20:50:09Z

(and @nodejs/collaborators )

MylesBorins · 2016-02-17T20:52:01Z

.github/ISSUE_TEMPLATE.md

+- [ ] solaris
+- [ ] other _(please specify which)_
+
+### Core part (if known)


Core subsystem?

MylesBorins · 2016-02-17T20:52:40Z

It would be nice to include a section about whether or not the PR should be considered for LTS

indutny · 2016-02-17T20:53:33Z

@thealphanerd Done.

indutny · 2016-02-17T20:54:10Z

I have also re-ordered versions so that most recent ones will appear first.

tflanagan · 2016-02-17T20:55:40Z

.github/PULL_REQUEST_TEMPLATE.md

+### SemVer
+
+What semver change does this change require?
+


Add (see http://semver.org/)?

Fishrock123 · 2016-02-17T20:58:55Z

I think this is generally too big and may be even more daunting to newcomers. I'm only in favor of something minimal.

Trott · 2016-02-17T20:59:54Z

The more stuff in the template, the larger the barrier for people to submit issues. That may arguably be a good thing, if it prevents people from submitting exceptionally low-quality bug reports. But I would encourage starting with something as minimal as can be tolerated.

EDIT: Looks like Jeremiah beat me to it by 30 seconds.

tflanagan · 2016-02-17T21:00:15Z

.github/ISSUE_TEMPLATE.md

+- [ ] cluster
+- [ ] crypto
+- [ ] dgram
+- [ ] dns


indutny · 2016-02-17T21:01:28Z

@Fishrock123 @Trott this mean that someone from the @nodejs/collaborators will have to ask these questions anyway, and it means both time spent on this, and someone's efforts.

While templates are looking a bit big indeed, the actual amount of data that I ask is pretty minimal.

mscdex · 2016-02-17T21:03:26Z

I do agree somewhat with @Fishrock123 and @Trott. Some users may not even know what core part(s) to put a check in (either because it's not obvious initially or the submitter is not familiar enough with node core to be able to comfortably choose).

indutny · 2016-02-17T21:03:28Z

@Fishrock123 @Trott being more specific, there are just three questions when opening an issue, and it is asking for details (which are required anyway), and a test case.

indutny · 2016-02-17T21:03:48Z

@mscdex that's why it is says (if known)...

Fishrock123 · 2016-02-17T21:05:41Z

usually we don't have to ask for subsystem, and since we have to tag it anyways, I think that should be let off for if we have caine do it someday.

indutny · 2016-02-17T21:07:27Z

@Fishrock123 caine will have to ask for it anyway. The only alternative is a machine learning technique to figure out subsystem automatically.

Fishrock123 · 2016-02-17T21:07:31Z

.github/PULL_REQUEST_TEMPLATE.md

+
+### LTS
+
+Should this patch be backported to:


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?

This sounds reasonable. I will change it once we will decide on general form of these files :)

Trott · 2016-02-17T21:20:34Z

Just a suggestion, feel free to ignore:

Maybe we should figure out use cases for issues to make sure the template isn't making too many assumptions.

In other words, issues aren't just used to report defects. They are used to submit feature requests. They are used to ask questions, at least some of which are appropriate for this repo. And they are used for process (such as issues for meetings). They are used to notify collaborators of things (such as that the CI is locked down for a period of time).

It might be useful to catalog these use cases (and maybe I've done it above), and figure out if the template needs to be generic enough for some of them. (On the one hand, I think someone who is using an issue for a meeting agenda can be expected to delete the template. On the other hand, someone submitting a feature request, maybe not as much?)

Trott · 2016-02-17T21:24:01Z

For the record, I am very much in favor of templates. And on balance, I like these templates. I would just be more comfortable starting as small as possible and adding items as we see how it works in the real world.

mscdex · 2016-02-23T02:14:25Z

.github/PULL_REQUEST_TEMPLATE.md

+
+- [ ] Does `make -j8 test` (UNIX) or `vcbuild test nosign` (Windows) pass with
+  this change (including linting)?
+- [ ] Is the commit message formatted according to [CONTRIBUTING.md][0]


nit: missing ? at the end

indutny · 2016-02-23T02:39:33Z

@mscdex all fixed, PTAL

mscdex · 2016-02-23T02:52:55Z

.github/PULL_REQUEST_TEMPLATE.md

+  this change (including linting)?
+- [ ] Is the commit message formatted according to [CONTRIBUTING.md][0]?
+- [ ] If this change fixes a bug (or a performance problem), is a regression
+  test included (or a benchmark)?


might move (or a benchmark) before included.

mscdex · 2016-02-23T02:55:26Z

One last nit, but otherwise LGTM

indutny · 2016-02-23T03:50:45Z

@mscdex thank you!

@mikeal do you want to take a look at it? If no - I will be glad to land it as it is (it seems that we are very close to consensus on current version of it). Thanks!

Fishrock123 · 2016-02-23T03:57:04Z

Afaik @nebrius has been meaning to chime in but was busy, maybe wait another day?

indutny · 2016-02-23T03:59:46Z

@Fishrock123 idk, these templates are going to evolve anyway. We may continue discussion on a follow-up fix, I guess? Current version should be good enough for several days of UX testing.

indutny · 2016-02-23T05:13:44Z

Alright, going to land it. Thanks everyone!

indutny · 2016-02-23T05:29:25Z

Landed in b4c6c5d, thank you everyone!

indutny · 2016-02-23T05:30:27Z

Gosh, forgot to include PR-URL.

Fix: #5246 PR-URL: #5291 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Steven R. Loomis <srloomis@us.ibm.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>

indutny · 2016-02-23T05:37:44Z

Sorry, had to force push in 81e35b5

Fix: #5246 PR-URL: #5291 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Steven R. Loomis <srloomis@us.ibm.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>

Refs: nodejs/node#5291

Refs: nodejs/node#5291 PR-URL: #22 Reviewed-By: Chengqiang Chen <602196490@qq.com> Reviewed-By: Cleverboy32 <wyzhdu@163.com>

github: add issue and pull request templates

f95cc96

Fix: nodejs#5246

indutny added the meta Issues and PRs related to the general management of the project. label Feb 17, 2016

MylesBorins reviewed Feb 17, 2016
View reviewed changes

.github/ISSUE_TEMPLATE.md

- [ ] solaris

- [ ] other _(please specify which)_

### Core part (if known)

Copy link

Contributor

MylesBorins Feb 17, 2016

Choose a reason for hiding this comment

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

Core subsystem?

github: improve

a96607b

subsystem

c245418

tflanagan reviewed Feb 17, 2016
View reviewed changes

.github/PULL_REQUEST_TEMPLATE.md

### SemVer

What semver change does this change require?

Copy link

Contributor

tflanagan Feb 17, 2016

Choose a reason for hiding this comment

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

Add (see http://semver.org/)?

Trott mentioned this pull request Feb 17, 2016

Implement issue templates? #5288

Closed

tflanagan reviewed Feb 17, 2016
View reviewed changes

indutny added 2 commits February 17, 2016 16:02

pr: doc/semver link

260d590

issue: doc subsystem

cc21cc0

Fishrock123 reviewed Feb 17, 2016
View reviewed changes

mscdex reviewed Feb 23, 2016
View reviewed changes

github: fix nits

ab479c5

mscdex reviewed Feb 23, 2016
View reviewed changes

github: last nit

3ba6e11

indutny closed this Feb 23, 2016

indutny deleted the fix/gh-5246 branch February 23, 2016 05:29

rvagg added dont-land-on-v5.x labels Feb 27, 2016

XadillaX added a commit to XadillaX/akyuu that referenced this pull request Jul 5, 2017

github: add issue and pull request templates

c6a3004

Refs: nodejs/node#5291

XadillaX mentioned this pull request Jul 5, 2017

github: add issue and pull request templates akyuujs/akyuu#22

Closed

2 tasks

sallen450 pushed a commit to akyuujs/akyuu that referenced this pull request Jul 6, 2017

github: add issue and pull request templates

8559ae4

Refs: nodejs/node#5291 PR-URL: #22 Reviewed-By: Chengqiang Chen <602196490@qq.com> Reviewed-By: Cleverboy32 <wyzhdu@163.com>

XadillaX added a commit to akyuujs/akyuu that referenced this pull request Jul 6, 2017

github: add issue and pull request templates

09d22e2

Refs: nodejs/node#5291 PR-URL: #22 Reviewed-By: Chengqiang Chen <602196490@qq.com> Reviewed-By: Cleverboy32 <wyzhdu@163.com>

XadillaX added a commit to akyuujs/akyuu that referenced this pull request Jul 6, 2017

github: add issue and pull request templates

81cf929

Refs: nodejs/node#5291 PR-URL: #22 Reviewed-By: Chengqiang Chen <602196490@qq.com> Reviewed-By: Cleverboy32 <wyzhdu@163.com>

github: add issue and pull request templates #5291

github: add issue and pull request templates #5291

Conversation

indutny commented Feb 17, 2016

indutny commented Feb 17, 2016

indutny commented Feb 17, 2016

indutny commented Feb 17, 2016

Choose a reason for hiding this comment

MylesBorins commented Feb 17, 2016

indutny commented Feb 17, 2016

indutny commented Feb 17, 2016

Choose a reason for hiding this comment

Fishrock123 commented Feb 17, 2016

Trott commented Feb 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

indutny commented Feb 17, 2016

mscdex commented Feb 17, 2016

indutny commented Feb 17, 2016

indutny commented Feb 17, 2016

Fishrock123 commented Feb 17, 2016

indutny commented Feb 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trott commented Feb 17, 2016

Trott commented Feb 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

indutny commented Feb 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mscdex commented Feb 23, 2016

indutny commented Feb 23, 2016

Fishrock123 commented Feb 23, 2016

indutny commented Feb 23, 2016

indutny commented Feb 23, 2016

indutny commented Feb 23, 2016

indutny commented Feb 23, 2016

indutny commented Feb 23, 2016