FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: make node(1) more consistent with tradition by strugee · Pull Request #8902 · nodejs/node · GitHub

/ node Public

doc: make node(1) more consistent with tradition - #8902

Closed
strugee wants to merge 1 commit into
nodejs:masterfrom
strugee:better-manpage
ConversationCommits (1)ChecksFiles changed
Closed

doc: make node(1) more consistent with tradition#8902
strugee wants to merge 1 commit into
nodejs:masterfrom
strugee:better-manpage

Conversation

strugee commented Oct 3, 2016

Copy link
Copy Markdown
Contributor
Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change
  • Removed loads of whitespace
  • Added traditional BUGS and AUTHORS sections
  • Renamed ENVIRONMENT VARIABLES to ENVIRONMENT, as is traditional

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.

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 3, 2016

bnoordhuis commented Oct 3, 2016

Copy link
Copy Markdown
Member

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.

addaleax reviewed Oct 3, 2016
View reviewed changes
Comment thread doc/node.1 Outdated

addaleax Oct 3, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should this be “blob”?

strugee Oct 3, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nice catch! Fixed.

Fishrock123 commented Oct 3, 2016

Copy link
Copy Markdown
Contributor

I don't agree with the ./" -- imo this will only make it harder for people less familiar to contribute to this. I suggest removing those.

Fishrock123 suggested changes Oct 3, 2016
View reviewed changes
Comment thread doc/node.1 Outdated

Fishrock123 Oct 3, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Effectively written by many others too. I would just link to the AUTHORS.

Comment thread doc/node.1 Outdated

Fishrock123 Oct 3, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

same, maybe just link the issue tracker? it's also linked below though....

imo, just remove

strugee commented Oct 3, 2016

Copy link
Copy Markdown
Contributor Author

@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 .\"s and added a COPYRIGHT section

Fishrock123 suggested changes Oct 4, 2016
View reviewed changes
Comment thread doc/node.1 Outdated

Fishrock123 Oct 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread doc/node.1 Outdated

Fishrock123 Oct 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Add spaces between please (see above note)

Comment thread doc/node.1 Outdated

Fishrock123 Oct 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think this correct -- all rights are not reserved, it is licensed under MIT.

strugee Oct 4, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Honestly, I thought so too. But that's what LICENSE says. Does that need to be changed too?

strugee Oct 4, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@Fishrock123 changed this despite the inconsistency with LICENSE.

Comment thread doc/node.1 Outdated

Fishrock123 Oct 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think the previous wording is more obvious.

strugee commented Oct 4, 2016

Copy link
Copy Markdown
Contributor Author

All issues fixed, including the "All rights reserved" section (even though it's inconsistent with LICENSE).

jasnell approved these changes Oct 6, 2016
View reviewed changes

jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Couple of nits I'd like to see fixed up but otherwise LGTM

Comment thread doc/node.1 Outdated

jasnell Oct 6, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

s/Node/Node.js

Comment thread doc/node.1 Outdated

jasnell Oct 6, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Fishrock123 reviewed Oct 6, 2016
View reviewed changes
Comment thread doc/node.1 Outdated

Fishrock123 Oct 6, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

could please add spaces here? looks good other than that

strugee commented Oct 6, 2016

Copy link
Copy Markdown
Contributor Author

👍 all issues fixed; this is ready to merge

jasnell reviewed Oct 7, 2016
View reviewed changes
Comment thread doc/node.1 Outdated

jasnell Oct 7, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

nit: long line here. could be fixed when landing.

strugee Oct 8, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed

doc: make node(1) more consistent with tradition
9d477f5 
* Added traditional BUGS, AUTHORS and COPYRIGHT sections
* Fixed some minor issues with the IRC links
jasnell pushed a commit that referenced this pull request Oct 8, 2016
doc: make node(1) more consistent with tradition
90cd39f 
* Added traditional BUGS, AUTHORS and COPYRIGHT sections
* Fixed some minor issues with the IRC links

PR-URL: #8902
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented Oct 8, 2016

Copy link
Copy Markdown
Member

Landed in 90cd39f. Thank you!

jasnell closed this Oct 8, 2016

strugee commented Oct 8, 2016

Copy link
Copy Markdown
Contributor Author

Thanks very much! 👍

jasnell pushed a commit that referenced this pull request Oct 10, 2016
doc: make node(1) more consistent with tradition
77f8bdd 
* Added traditional BUGS, AUTHORS and COPYRIGHT sections
* Fixed some minor issues with the IRC links

PR-URL: #8902
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
doc: make node(1) more consistent with tradition
72ed6aa 
* Added traditional BUGS, AUTHORS and COPYRIGHT sections
* Fixed some minor issues with the IRC links

PR-URL: #8902
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
doc: make node(1) more consistent with tradition
fc77cbb 
* Added traditional BUGS, AUTHORS and COPYRIGHT sections
* Fixed some minor issues with the IRC links

PR-URL: #8902
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Nov 22, 2016
Merged
strugee deleted the better-manpage branch August 26, 2017 22:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
addaleax addaleax left review comments
+1 more reviewer
Fishrock123 Fishrock123 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL