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

update N-API docs #27745

Closed
wants to merge 2 commits into from
Closed

update N-API docs #27745

wants to merge 2 commits into from

Conversation

richardlau
Copy link
Member

  • doc,n-api: fix introduced_in metadata
    Node.js v7.10.0 did not contain N-API. Update the introduced_in
    metadata to prevent a broken 7.x "View another version" link in the
    N-API docs.
  • doc,n-api: update N-API version matrix for v12.x

The first commit ("fix introduced_in metadata") fixes a 404 not found link
(https://nodejs.org/docs/latest-v7.x/api/n-api.html) which was disovered
in #27267 (https://travis-ci.com/nodejs/node/jobs/193473667#L1277).
image

As seen in the version matrix, N-API was backported to Node.js 6. I set
introduced_in for v8.0.0 based on Node.js 6 being End-of-Life and
setting to a 6.x release would still result in the broken link to 7.x due to
the simplistic way the "View another version" links are generated:

node/tools/doc/html.js

Lines 417 to 423 in f4572cc

function isDocInVersion(version) {
const [versionMajor, versionMinor] = version.num.split('.').map(Number);
if (docCreatedMajor > versionMajor) return false;
if (docCreatedMajor < versionMajor) return true;
if (Number.isNaN(versionMinor)) return true;
return docCreatedMinor <= versionMinor;
}

For the version matrix the non-12.x rows of the N-API version 4 column
should be handled in #27567 (raised by a first time contributor).

cc @nodejs/n-api @nodejs/documentation

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

Node.js v7.10.0 did not contain N-API. Update the `introduced_in`
metadata to prevent a broken 7.x "View another version" link in the
N-API docs.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. labels May 17, 2019
@BridgeAR
Copy link
Member

I wonder if we should add another optional information to skip some versions. We could also automate it by checking the release dates of each version.

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 17, 2019
@mhdawson
Copy link
Member

@BridgeAR I agree being able to skip versions would be good.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

I hope this does not cause confusion, but with 6.x being EOL I'm willing to give it a try.

@addaleax
Copy link
Member

Landed in b3ffd1f 98a552d

@addaleax addaleax closed this May 19, 2019
addaleax pushed a commit that referenced this pull request May 19, 2019
Node.js v7.10.0 did not contain N-API. Update the `introduced_in`
metadata to prevent a broken 7.x "View another version" link in the
N-API docs.

PR-URL: #27745
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request May 19, 2019
PR-URL: #27745
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request May 20, 2019
Node.js v7.10.0 did not contain N-API. Update the `introduced_in`
metadata to prevent a broken 7.x "View another version" link in the
N-API docs.

PR-URL: #27745
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request May 20, 2019
PR-URL: #27745
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@BridgeAR BridgeAR mentioned this pull request May 21, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants