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: update N-API version matrix #29461

Conversation

gabrielschulhof
Copy link
Contributor

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

@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 Sep 5, 2019
doc/api/n-api.md Outdated
| | 1 | 2 | 3 | 4 | 5 |
|:-----:|:-------:|:--------:|:--------:|:--------:|:---------:|
| v6.x | | | v6.14.2* | | |
| v8.x | v8.0.0* | v8.10.0* | v8.11.2 | v8.16.0 | REPLACEME |
Copy link
Member

Choose a reason for hiding this comment

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

Should these all be REPLACME ? I know we use that normally for when versions need to inserted but I think that might make sense for only master, which is not listed in the table at all . Maybe we need to add v13.x with the replacement and leave the others blank until the backports are complete?

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 wasn't sure how to mark them either. Nevertheless, we need to replace the appropriate token on all branches whenever N-API 5 appears in any of them.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, its just that I think there is some tooling related to REPLACEME and releases. @BethGriggs might know the specifics off the top of her head.

Copy link
Member

Choose a reason for hiding this comment

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

@nodejs/releasers replace all of the REPLACEME placeholders when a release is cut: https://github.com/nodejs/node/blob/master/doc/releases.md#step-3-update-any-replaceme-and-dep00xx-tags-in-the-docs

If this landed on master and was then cherry-picked into 12.x then the entire column would be replaced with the 12.x release which would not be the desired result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see, so, for each backport, the appropriate row must be marked as REPLACEME.

@gabrielschulhof
Copy link
Contributor Author

@richardlau @mhdawson I have updated the matrix, adding row "v13.x" and putting "REPLACEME" for N-API version 5.

@richardlau
Copy link
Member

I've stuck don't-land-on labels as this would need adjusting for each release backported to (i.e. a different row would need to be modified) so should be manually backported.

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.

LGTM

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Sep 8, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/25390/

Since this is a doc-only change, lite CI would be enough, so I'm going to ignore the fact that we don't have any LinuxONE machines available right now and say "linter passed, tests passed on other platforms, this is good to land".

@Trott
Copy link
Member

Trott commented Sep 8, 2019

Landed in eaa9f83

@Trott Trott closed this Sep 8, 2019
Trott pushed a commit that referenced this pull request Sep 8, 2019
PR-URL: #29461
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Sep 18, 2019
PR-URL: nodejs#29461
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Sep 21, 2019
PR-URL: nodejs#29461
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
PR-URL: #29461
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Sep 25, 2019
BethGriggs pushed a commit that referenced this pull request Oct 1, 2019
PR-URL: #29461
Backport-PR-URL: #29643
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Oct 7, 2019
@gabrielschulhof gabrielschulhof deleted the update-napi-version-matrix branch April 21, 2020 02:58
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. 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