Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
doc: trim GitHub template comments#6755
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
.github/ISSUE_TEMPLATE.md Outdated
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.
What about capitalizing the field names here to match the actual content (and to make them stand out better)?
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.
@mscdex Good idea. Done.
jbergstroem commented May 14, 2016
LGTM |
.github/PULL_REQUEST_TEMPLATE.md Outdated
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.
While you're here: "Run the test suite with:" and "on UNIX"?
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.
make test already includes lint. make -j4 test
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.
While you're here: "Run the test suite with:" and "on UNIX"?
Done and done.
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.
make test already includes lint. make -j4 test
lint removed
bnoordhuis commented May 14, 2016
LGTM with a suggestion. |
Fishrock123 commented May 14, 2016
Can we get rid of the blank lines between the headings and the comments? ### thing<!-- description --> |
jasnell commented May 14, 2016
Rubber stamp LGTM. |
Trott commented May 15, 2016
We sure can! Done! |
Make the comments in the GitHub templates slightly more concise.
rvagg commented May 15, 2016
lgtm The last para is horribly verbose, mostly redundant and simply sounds like someone's trying too hard to be nice and unfortunately it becomes long enough that it's likely to be left unread by many.
[I'd rather remove it, but] it could become something like:
I'd love to see data on whether any of the text in these templates has changed behaviour at all. It's probably an unreasonable request but I'm highly skeptical that we're getting much value out of these and it's just contributing to the noise and raising barriers to entry. |
Fishrock123 commented May 15, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@rvagg We do actually usually find these at least attempted to be filled in as far as I have seen.. I think we should minimize the language and make it as concise as possible but I think keeping it is also a good idea. |
jasnell commented May 17, 2016
Still LGTM. Would love it if we can find ways of trimming more. |
Make the comments in the GitHub templates slightly more concise. PR-URL: nodejs#6755 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
Trott commented May 17, 2016
Landed in 42ede93 |
Make the comments in the GitHub templates slightly more concise. PR-URL: #6755 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
Checklist
Affected core subsystem(s)
doc
Description of change
Make the comments in the GitHub templates slightly more concise.
/cc @indutny