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

tools: adds doc links to the stability index reference #11664

Closed
wants to merge 1 commit into from
Closed

tools: adds doc links to the stability index reference #11664

wants to merge 1 commit into from

Conversation

michaelcox
Copy link
Contributor

@michaelcox michaelcox commented Mar 2, 2017

Checklist
  • make -j4 test (UNIX)
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

Description of Change

This modifies the script that generates the docs to create a static link from each Stability Index callout bar to the Stability Index explanations in documentation.html. This was a first-commit request from @Trott. Here is what he pitched to me:

Many APIs have a Stability Index associated with them. Check out https://nodejs.org/dist/latest-v7.x/docs/api/modules.html#modules_modules for an example (the Modules API is Locked, Stability Index 3).
It would be great if there could be a link in the Stability Index banner to the definition of the various Stability Indexes. If I don't know what "Locked" means, then I can click the link and find out.
Modify html.js to include such a link. This will involve some HTML of course, and probably some CSS and maybe some back-and-forth about usability/styling once the PR is open.

I looked for any precedent for styling, and the best I could find is that when features are deprecated, a simple link is put to the new API to use, inside the banner. This similarly just links the first part (e.g. Stability: 2) to the documentation.html page. I'm obviously open to other styling suggestions, but this seemed the most straightforward. There doesn't seem to be any precedent in the docs for other things like a ? icon or similar.

Also I'm just linking to the main anchor for the Stability Index explanations, rather than to a specific one. My thinking here is that there are only a handful of them, and they should all fit nicely above the fold for anyone reading that page. It didn't seem necessary to create additional anchors.

Note that I had to make a couple more variables in the commit here to make the tests pass with the 80 character line limit.

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Mar 2, 2017
@hiroppy
Copy link
Member

hiroppy commented Mar 2, 2017

I think that the commit message should be tools rather than doc;)

@michaelcox
Copy link
Contributor Author

Thanks - updated to be tools instead. Makes more sense. :)

@michaelcox michaelcox changed the title doc: adds links to the stability index reference tools: adds doc links to the stability index reference Mar 3, 2017
@addaleax addaleax added the doc Issues and PRs related to the documentations. label Mar 3, 2017
@hiroppy
Copy link
Member

hiroppy commented Mar 3, 2017

cc @nodejs/documentation

@Trott
Copy link
Member

Trott commented Mar 3, 2017

I don't think this file is exercised by any tests, but it is covered by lint rules, so here's a CI lint run for it: https://ci.nodejs.org/job/node-test-linter/7386/

text = text.replace(
STABILITY_TEXT_REG_EXP,
'<pre class="api_stability api_stability_$2">$1 $2$3</pre>'
`<pre class="${classNames}"><a href="${docsUrl}">$1 $2</a>$3
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I would prefer writing this as,

`<pre class="${classNames}">
    <a href="${docsUrl}">$1 $2</a>$3
 </pre>`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated to bring the closing </pre> to the same line, but I can't write it nested the way you have it because of the default formatting of a <pre> tag. Doing it the way you had it adds another 20px-ish left padding into the bar.

This modifies the script that generates the docs to create a
static link from each Stability Index callout bar to the
Stability Index explanations in documentation.html.
@fhinkel
Copy link
Member

fhinkel commented Mar 9, 2017

Thanks. Landed in dd76f5f

@fhinkel fhinkel closed this Mar 9, 2017
fhinkel pushed a commit that referenced this pull request Mar 9, 2017
This modifies the script that generates the docs
to create a static link from each Stability Index callout bar to
the Stability Index explanations in documentation.html.

PR-URL: #11664
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 13, 2017
This modifies the script that generates the docs
to create a static link from each Stability Index callout bar to
the Stability Index explanations in documentation.html.

PR-URL: nodejs#11664
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
This modifies the script that generates the docs
to create a static link from each Stability Index callout bar to
the Stability Index explanations in documentation.html.

PR-URL: nodejs#11664
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 17, 2017
This modifies the script that generates the docs
to create a static link from each Stability Index callout bar to
the Stability Index explanations in documentation.html.

PR-URL: #11664
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
This modifies the script that generates the docs
to create a static link from each Stability Index callout bar to
the Stability Index explanations in documentation.html.

PR-URL: #11664
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@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
This modifies the script that generates the docs
to create a static link from each Stability Index callout bar to
the Stability Index explanations in documentation.html.

PR-URL: nodejs/node#11664
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@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. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants