-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: make node(1) more consistent with tradition #8902
Conversation
The "has many bugs" in the BUGS section is too nondescript to be useful, I'd change it to a REPORTING BUGS section with a link to the bug tracker. I personally don't care for an AUTHORS section; it's not very relevant to readers and in our case it's misleading because Ryan hasn't worked on node for something like four years now. Suggestion: a LICENSE section might be a good addition. |
.\" | ||
.SH AUTHORS | ||
Written by Ryan Dahl and maintained by 1000+ contributors: | ||
.ur https://github.com/nodejs/node/blog/master/AUTHORS |
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 this be “blob”?
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.
Nice catch! Fixed.
I don't agree with the |
.ur https://github.com/nodejs/node/issues | ||
.\" | ||
.SH AUTHORS | ||
Written by Ryan Dahl and maintained by 1000+ contributors: |
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.
Effectively written by many others too. I would just link to the AUTHORS.
.\" | ||
.SH BUGS | ||
Node.js has many bugs; they are tracked in GitHub Issues: | ||
.ur https://github.com/nodejs/node/issues |
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.
same, maybe just link the issue tracker? it's also linked below though....
imo, just remove
f02d24b
to
93efcd8
Compare
@bnoordhuis @Fishrock123 changed the wording of the BUGS section to be more concise. If people still disagree with this I can just remove it entirely I also changed AUTHORS to not mention Ryan, got rid of the |
93efcd8
to
a84da3f
Compare
.\" Macro to display an underlined URL in bold | ||
.de ur | ||
.nr CF \\n(.f | ||
.ft 4 | ||
\\$1 | ||
.ft \\n(CF | ||
.. | ||
|
||
|
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.
Sorry, I meant not to touch these at all.
I know it's not "perfectly syntactically correct" but it is far more readable.
In my experience the problems with rendering are negligible if any for the potential cost of people having a hard time contributing.
.SH BUGS | ||
Bugs are tracked in GitHub Issues: | ||
.ur https://github.com/nodejs/node/issues | ||
.SH AUTHORS |
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.
Add spaces between please (see above note)
Written and maintained by 1000+ contributors: | ||
.ur https://github.com/nodejs/node/blob/master/AUTHORS | ||
.SH COPYRIGHT | ||
Copyright Node.js contributors. All rights reserved. Node.js is available under the MIT license. |
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.
I don't think this correct -- all rights are not reserved, it is licensed under MIT.
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.
Honestly, I thought so too. But that's what LICENSE says. Does that need to be changed too?
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.
@Fishrock123 changed this despite the inconsistency with LICENSE.
|
||
.SH ENVIRONMENT VARIABLES | ||
|
||
.SH ENVIRONMENT |
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.
I think the previous wording is more obvious.
a84da3f
to
22e5f96
Compare
All issues fixed, including the "All rights reserved" section (even though it's inconsistent with LICENSE). |
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.
Couple of nits I'd like to see fixed up but otherwise LGTM
@@ -219,5 +231,5 @@ Mailing list: | |||
IRC (general questions): | |||
.ur "chat.freenode.net #node.js" | |||
|
|||
IRC (node core development): | |||
IRC (Node core development): |
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.
s/Node/Node.js
@@ -219,5 +231,5 @@ Mailing list: | |||
IRC (general questions): | |||
.ur "chat.freenode.net #node.js" |
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.
Nit: Given that #node.js
is not yet considered an official resource managed by the project, a note indicating it's unofficial status might be worthwhile.
.SH BUGS | ||
Bugs are tracked in GitHub Issues: | ||
.ur https://github.com/nodejs/node/issues | ||
.SH AUTHORS |
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.
could please add spaces here? looks good other than that
22e5f96
to
deb7cd5
Compare
👍 all issues fixed; this is ready to merge |
.SH COPYRIGHT | ||
Copyright Node.js contributors. Node.js is available under the MIT license. | ||
|
||
Node.js also includes external libraries that are available under a variety of licenses. See |
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.
nit: long line here. could be fixed when landing.
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.
Fixed
deb7cd5
to
1a6c407
Compare
* Added traditional BUGS, AUTHORS and COPYRIGHT sections * Fixed some minor issues with the IRC links
1a6c407
to
9d477f5
Compare
* Added traditional BUGS, AUTHORS and COPYRIGHT sections * Fixed some minor issues with the IRC links PR-URL: #8902 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in 90cd39f. Thank you! |
Thanks very much! 👍 |
* Added traditional BUGS, AUTHORS and COPYRIGHT sections * Fixed some minor issues with the IRC links PR-URL: #8902 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
* Added traditional BUGS, AUTHORS and COPYRIGHT sections * Fixed some minor issues with the IRC links PR-URL: #8902 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
* Added traditional BUGS, AUTHORS and COPYRIGHT sections * Fixed some minor issues with the IRC links PR-URL: #8902 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
Affected core subsystem(s)
doc
Description of change
In particular I'm not sure if everyone will be cool with the BUGS text - I personally think it's good but if anyone disagrees I'm not super attached to it. Also, I put
.\"
s instead of outright removing the newlines because I think it's (slightly) more readable, but I can also get rid of those.