Skip to content
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: add table of contents to README.md #11635

Closed
wants to merge 2 commits into from

Conversation

Minimalistic
Copy link
Contributor

I have added a Table of Contents to the top of the readme which includes relative links to allow jumping to the appropriate section of the readme.

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

I have added a TOC to the top of the readme, relative links to allow jumping to the appropriate section of the readme.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 1, 2017
@Fishrock123
Copy link
Contributor

Ideally I'd like Resources for Newcomers to be above the TOC

@Fishrock123 Fishrock123 closed this Mar 1, 2017
@Fishrock123 Fishrock123 reopened this Mar 1, 2017
@Minimalistic
Copy link
Contributor Author

Certainly could reorganize it a bit, maybe integrate resources for newcomers to be a part of the initial description above the TOC?

@Trott
Copy link
Member

Trott commented Mar 2, 2017

I'm not excited about having Resources for Newcomers repeated twice in the document, nearly verbatim. It's misleading the way it's part of the table of contents, as the links do not go to other parts of the document. They go to external resources. That seems like it would violate user expectations.

  • Resources for Newcomers content should appear once in the doc. (I don't care if we move it higher in the doc, although that should probably be done in a different PR than a PR adding a ToC. It's unrelated.)

  • If "Resources for Newcomers" appears in the ToC, it should link to the section of the doc that contains Resources for Newcomers and not repeat the content of that section.

@Trott
Copy link
Member

Trott commented Mar 2, 2017

Other than my previous concern, +💯 to including a table of contents in this doc. 👍

@fhinkel
Copy link
Member

fhinkel commented Mar 26, 2017

Can we merge this?

@gibfahn
Copy link
Member

gibfahn commented Mar 26, 2017

Can we merge this?

Looks like @Trott's change needs to be implemented (ping @Minimalistic):

If "Resources for Newcomers" appears in the ToC, it should link to the section of the doc that contains Resources for Newcomers and not repeat the content of that section.


The other raised issue was @Fishrock123's :

Ideally I'd like Resources for Newcomers to be above the TOC

@Minimalistic if you're going to change that in this PR as well please do it as a separate commit, otherwise it can be done later.

Trott
Trott previously requested changes Mar 26, 2017
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

I'm not excited about having Resources for Newcomers repeated twice in the document, nearly verbatim. It's misleading the way it's part of the table of contents, as the links do not go to other parts of the document. They go to external resources. That seems like it would violate user expectations.

Resources for Newcomers content should appear once in the doc. (I don't care if we move it higher in the doc, although that should probably be done in a different PR than a PR adding a ToC. It's unrelated.)

If "Resources for Newcomers" appears in the ToC, it should link to the section of the doc that contains Resources for Newcomers and not repeat the content of that section.

Remove repetitious links in "Resources for Newcomers" in TOC
@Trott Trott dismissed their stale review March 28, 2017 04:43

Changes have been committed to address my concerns. (Thanks!)

jasnell pushed a commit that referenced this pull request Apr 4, 2017
PR-URL: #11635
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@jasnell jasnell changed the title Table of Contents and Relative Links doc: add table of contents to README.md Apr 4, 2017
@jasnell
Copy link
Member

jasnell commented Apr 4, 2017

Landed in f62d9fc

@jasnell jasnell closed this Apr 4, 2017
@jasnell jasnell mentioned this pull request Apr 4, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Apr 10, 2017
PR-URL: nodejs#11635
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
2 tasks
MylesBorins pushed a commit that referenced this pull request Apr 18, 2017
PR-URL: #11635
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@MylesBorins
Copy link
Contributor

This is really useful! Great work.

MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
PR-URL: #11635
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Apr 19, 2017
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
PR-URL: nodejs/node#11635
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants