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.

Sign up for GitHub

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

Jump to bottom

n-api: define release 6 #32058

Closed
wants to merge 1 commit into from
Closed

n-api: define release 6 #32058

wants to merge 1 commit into from

Conversation

gabrielschulhof
Copy link
Contributor

Mark all N-APIs that have been added since version 5 as stable.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 3, 2020
@gabrielschulhof gabrielschulhof added the node-api Issues and PRs related to the Node-API. label Mar 3, 2020
@gabrielschulhof gabrielschulhof force-pushed the n-api-6 branch 2 times, most recently from 5da4811 to b094860 Compare March 3, 2020 07:24
legendecas
legendecas approved these changes Mar 5, 2020
View reviewed changes
Copy link
Member

@legendecas legendecas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM % napi_create_bigint_uint64 is missing in the changed docs.

@gabrielschulhof gabrielschulhof mentioned this pull request Mar 5, 2020
Closed
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@legendecas
Copy link
Member

Maybe I'm not been clear about the issue. https://github.com/nodejs/node/blob/master/doc/api/n-api.md#napi_create_bigint_uint64 there is a doc on napi_create_bigint_uint64 but the stability and version status have not been changed in the PR.

@nodejs-github-bot
Copy link
Collaborator

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

@gabrielschulhof
Copy link
Contributor Author

@legendecas fixed.

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@gabrielschulhof
Copy link
Contributor Author

Rebased.

@nodejs-github-bot
Copy link
Collaborator

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

mhdawson
mhdawson reviewed Mar 6, 2020
View reviewed changes
src/js_native_api.h Outdated
@@ -531,7 +532,7 @@ NAPI_EXTERN napi_status napi_detach_arraybuffer(napi_env env,
NAPI_EXTERN napi_status napi_is_detached_arraybuffer(napi_env env,
napi_value value,
bool* result);
#endif // NAPI_EXPERIMENTAL
#endif // NAPI_VERSION >= 6
Copy link
Member

Choose a reason for hiding this comment

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

I don't see napi_is_detached_arraybuffer, napi_detach_arraybuffer in the 10.x staging branch and a quick look at PR's I don't see anything labelled with napi that would add them.

Did we intend to add them to NAPI_VERSION 6? I'm thinking no (unless I've missed the PR where they will be added) since we were waiting for other PR to land in 10.x first.

mhdawson
mhdawson requested changes Mar 6, 2020
View reviewed changes
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.

Requesting changes until my question is resolved

@gabrielschulhof
Copy link
Contributor Author

@mhdawson good point. Looks like we need to do some unifying of the release lines first. As it stands, we have the followings:

  • v12.x has
    • bigint
    • instance_data
    • detach
  • v10.x has
    • bigint
    • instance_data
    • get_all_property_names

So we need to backport get_all_property_names to v12.x, and detach to v10.x.

@gabrielschulhof
Copy link
Contributor Author

detach is unavailable in 10.

@gabrielschulhof
Copy link
Contributor Author

get_all_properties applies cleanly to v12.x-staging.

@gabrielschulhof gabrielschulhof mentioned this pull request Mar 7, 2020
Closed
3 tasks
@legendecas
n-api: define release 6
9f588c1 
Mark all N-APIs that have been added since version 5 as stable.

Fixes: nodejs/abi-stable-node#393
Co-Authored-By: legendecas <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
@gabrielschulhof
Copy link
Contributor Author

@mhdawson I removed the portion of the PR that marks the detach-related N-APIs as stable.

MylesBorins added a commit that referenced this pull request Mar 12, 2020
@MylesBorins
2020-03-11 Version 13.11.0 (Current)
4a53612 
Notable changes:

* async_hooks:
  - add sync enterWith to ALS (Stephen Belanger)
    #31945
* cli:
  - allow --jitless V8 flag in NODE\_OPTIONS (Andrew Neitsch)
    #32100
* fs:
  - return first folder made by mkdir recursive (Benjamin Coe)
    #31530
* n-api:
  - define release 6 (Gabriel Schulhof)
    #32058
* src:
  - create a getter for kernel version (Juan José Arboleda)
    #31732
* wasi:
  - add returnOnExit option (Colin Ihrig)
    #32101

PR-URL: #32185
MylesBorins added a commit that referenced this pull request Mar 12, 2020
@MylesBorins
2020-03-11 Version 13.11.0 (Current)
5bfbd75 
Notable changes:

* async_hooks:
  - add sync enterWith to ALS (Stephen Belanger)
    #31945
* cli:
  - allow --jitless V8 flag in NODE\_OPTIONS (Andrew Neitsch)
    #32100
* fs:
  - return first folder made by mkdir recursive (Benjamin Coe)
    #31530
* n-api:
  - define release 6 (Gabriel Schulhof)
    #32058
* src:
  - create a getter for kernel version (Juan José Arboleda)
    #31732
* wasi:
  - add returnOnExit option (Colin Ihrig)
    #32101

PR-URL: #32185
@mhdawson
Copy link
Member

@myles I believe so.

@mhdawson mhdawson added the semver-minor PRs that contain new features and should be released in the next minor version. label Mar 12, 2020
@gabrielschulhof
Copy link
Contributor Author

@mhdawson @MylesBorins N-API 5 was not semver-minor.

@gabrielschulhof
Copy link
Contributor Author

... at least it was not marked as such in the PR.

@MylesBorins
Copy link
Contributor

MylesBorins commented Mar 14, 2020 via email

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Mar 25, 2020
@legendecas
n-api: define release 6
e1e2c2d 
Mark all N-APIs that have been added since version 5 as stable.

PR-URL: nodejs#32058
Fixes: nodejs/abi-stable-node#393
Co-Authored-By: legendecas <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 2, 2020
@legendecas
n-api: define release 6
3a3c011 
Mark all N-APIs that have been added since version 5 as stable.

PR-URL: nodejs#32058
Fixes: nodejs/abi-stable-node#393
Co-Authored-By: legendecas <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 6, 2020
@legendecas @BethGriggs
n-api: define release 6
e9c590e 
Mark all N-APIs that have been added since version 5 as stable.

PR-URL: #32058
Backport-PR-URL: #32488
Fixes: nodejs/abi-stable-node#393
Co-Authored-By: legendecas <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
BethGriggs added a commit that referenced this pull request Apr 6, 2020
@BethGriggs
2020-04-07, Version 10.20.0 'Dubnium' (LTS)
0326fc1 
Notable changes:

- buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc)
  [#19691](#19691)
- build: macOS package notarization (Rod Vagg)
  [#31459](#31459)
- deps:
  - update npm to 6.14.3 (Myles Borins)
    [#32368](#32368)
  - upgrade openssl sources to 1.1.1e (Hassaan Pasha)
    [#32328](#32328)
  - upgrade to libuv 1.34.2 (cjihrig)
    [#31477](#31477)
- n-api:
  - add napi\_get\_all\_property\_names (himself65)
    [#30006](#30006)
  - add APIs for per-instance state management (Gabriel Schulhof)
    [#28682](#28682)
  - define release 6
    [#32058](#32058)
  - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen)
    [#26128](#26128)
- tls:
  - expose keylog event on TLSSocket (Alba Mendez)
    [#27654](#27654)
  - support TLS min/max protocol defaults in CLI (Sam Roberts)
    [#27946](#27946)
- url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig)
  [#26226](#26226)

PR-URL: #31984
@BethGriggs BethGriggs mentioned this pull request Apr 6, 2020
Merged
BethGriggs added a commit that referenced this pull request Apr 7, 2020
@BethGriggs
2020-04-07, Version 10.20.0 'Dubnium' (LTS)
d5f87c5 
macOS package notarization and a change in builder configuration

The macOS binaries for this release, and future 10.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing .pkg files on macOS
10.15 and later. Previous builds of Node.js 10.x were compiled on macOS
10.7 (Lion). As binaries are still being compiled to support a minimum
of macOS 10.7 (Lion) we do not anticipate this having a negative impact
on Node.js 10.x users with older versions of macOS.

Notable changes:

- buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc)
  [#19691](#19691)
- build: macOS package notarization (Rod Vagg)
  [#31459](#31459)
- deps:
  - update npm to 6.14.3 (Myles Borins)
    [#32368](#32368)
  - upgrade openssl sources to 1.1.1e (Hassaan Pasha)
    [#32328](#32328)
  - upgrade to libuv 1.34.2 (cjihrig)
    [#31477](#31477)
- n-api:
  - add napi\_get\_all\_property\_names (himself65)
    [#30006](#30006)
  - add APIs for per-instance state management (Gabriel Schulhof)
    [#28682](#28682)
  - define release 6
    [#32058](#32058)
  - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen)
    [#26128](#26128)
- tls:
  - expose keylog event on TLSSocket (Alba Mendez)
    [#27654](#27654)
  - support TLS min/max protocol defaults in CLI (Sam Roberts)
    [#27946](#27946)
- url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig)
  [#26226](#26226)

PR-URL: #31984
BethGriggs added a commit that referenced this pull request Apr 7, 2020
@BethGriggs
2020-04-07, Version 10.20.0 'Dubnium' (LTS)
b6d4334 
macOS package notarization and a change in builder configuration

The macOS binaries for this release, and future 10.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing .pkg files on macOS
10.15 and later. Previous builds of Node.js 10.x were compiled on macOS
10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion).
As binaries are still being compiled to support a minimum of macOS 10.7
(Lion) we do not anticipate this having a negative impact on Node.js
10.x users with older versions of macOS.

Notable changes:

- buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc)
  [#19691](#19691)
- build: macOS package notarization (Rod Vagg)
  [#31459](#31459)
- deps:
  - update npm to 6.14.3 (Myles Borins)
    [#32368](#32368)
  - upgrade openssl sources to 1.1.1e (Hassaan Pasha)
    [#32328](#32328)
  - upgrade to libuv 1.34.2 (cjihrig)
    [#31477](#31477)
- n-api:
  - add napi\_get\_all\_property\_names (himself65)
    [#30006](#30006)
  - add APIs for per-instance state management (Gabriel Schulhof)
    [#28682](#28682)
  - define release 6
    [#32058](#32058)
  - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen)
    [#26128](#26128)
- tls:
  - expose keylog event on TLSSocket (Alba Mendez)
    [#27654](#27654)
  - support TLS min/max protocol defaults in CLI (Sam Roberts)
    [#27946](#27946)
- url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig)
  [#26226](#26226)

PR-URL: #31984
BethGriggs added a commit that referenced this pull request Apr 8, 2020
@BethGriggs
2020-04-08, Version 10.20.0 'Dubnium' (LTS)
246eede 
macOS package notarization and a change in builder configuration

The macOS binaries for this release, and future 10.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing .pkg files on macOS
10.15 and later. Previous builds of Node.js 10.x were compiled on macOS
10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion).
As binaries are still being compiled to support a minimum of macOS 10.7
(Lion) we do not anticipate this having a negative impact on Node.js
10.x users with older versions of macOS.

Notable changes:

- buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc)
  [#19691](#19691)
- build: macOS package notarization (Rod Vagg)
  [#31459](#31459)
- deps:
  - update npm to 6.14.3 (Myles Borins)
    [#32368](#32368)
  - upgrade openssl sources to 1.1.1e (Hassaan Pasha)
    [#32328](#32328)
  - upgrade to libuv 1.34.2 (cjihrig)
    [#31477](#31477)
- n-api:
  - add napi\_get\_all\_property\_names (himself65)
    [#30006](#30006)
  - add APIs for per-instance state management (Gabriel Schulhof)
    [#28682](#28682)
  - define release 6
    [#32058](#32058)
  - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen)
    [#26128](#26128)
- tls:
  - expose keylog event on TLSSocket (Alba Mendez)
    [#27654](#27654)
  - support TLS min/max protocol defaults in CLI (Sam Roberts)
    [#27946](#27946)
- url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig)
  [#26226](#26226)

PR-URL: #31984
BethGriggs added a commit that referenced this pull request Apr 8, 2020
@BethGriggs
2020-04-08, Version 10.20.0 'Dubnium' (LTS)
9b46e3f 
macOS package notarization and a change in builder configuration

The macOS binaries for this release, and future 10.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing .pkg files on macOS
10.15 and later. Previous builds of Node.js 10.x were compiled on macOS
10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion).
As binaries are still being compiled to support a minimum of macOS 10.7
(Lion) we do not anticipate this having a negative impact on Node.js
10.x users with older versions of macOS.

Notable changes:

- buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc)
  [#19691](#19691)
- build: macOS package notarization (Rod Vagg)
  [#31459](#31459)
- deps:
  - update npm to 6.14.3 (Myles Borins)
    [#32368](#32368)
  - upgrade openssl sources to 1.1.1e (Hassaan Pasha)
    [#32328](#32328)
  - upgrade to libuv 1.34.2 (cjihrig)
    [#31477](#31477)
- n-api:
  - add napi\_get\_all\_property\_names (himself65)
    [#30006](#30006)
  - add APIs for per-instance state management (Gabriel Schulhof)
    [#28682](#28682)
  - define release 6
    [#32058](#32058)
  - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen)
    [#26128](#26128)
- tls:
  - expose keylog event on TLSSocket (Alba Mendez)
    [#27654](#27654)
  - support TLS min/max protocol defaults in CLI (Sam Roberts)
    [#27946](#27946)
- url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig)
  [#26226](#26226)

PR-URL: #31984
BethGriggs added a commit that referenced this pull request Apr 14, 2020
@BethGriggs
2020-04-08, Version 10.20.0 'Dubnium' (LTS)
8d8095f 
macOS package notarization and a change in builder configuration

The macOS binaries for this release, and future 10.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing .pkg files on macOS
10.15 and later. Previous builds of Node.js 10.x were compiled on macOS
10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion).
As binaries are still being compiled to support a minimum of macOS 10.7
(Lion) we do not anticipate this having a negative impact on Node.js
10.x users with older versions of macOS.

Notable changes:

- buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc)
  [#19691](#19691)
- build: macOS package notarization (Rod Vagg)
  [#31459](#31459)
- deps:
  - update npm to 6.14.3 (Myles Borins)
    [#32368](#32368)
  - upgrade openssl sources to 1.1.1e (Hassaan Pasha)
    [#32328](#32328)
  - upgrade to libuv 1.34.2 (cjihrig)
    [#31477](#31477)
- n-api:
  - add napi\_get\_all\_property\_names (himself65)
    [#30006](#30006)
  - add APIs for per-instance state management (Gabriel Schulhof)
    [#28682](#28682)
  - define release 6
    [#32058](#32058)
  - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen)
    [#26128](#26128)
- tls:
  - expose keylog event on TLSSocket (Alba Mendez)
    [#27654](#27654)
  - support TLS min/max protocol defaults in CLI (Sam Roberts)
    [#27946](#27946)
- url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig)
  [#26226](#26226)

PR-URL: #31984
@targos targos added backported-to-v12.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@legendecas @targos
n-api: define release 6
12b3fca 
Mark all N-APIs that have been added since version 5 as stable.

Backport-PR-URL: nodejs#32482
PR-URL: nodejs#32058
Fixes: nodejs/abi-stable-node#393
Co-Authored-By: legendecas <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
@legendecas @targos
n-api: define release 6
e22d853 
Mark all N-APIs that have been added since version 5 as stable.

Backport-PR-URL: #32482
PR-URL: #32058
Fixes: nodejs/abi-stable-node#393
Co-Authored-By: legendecas <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@targos targos mentioned this pull request May 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@legendecas legendecas legendecas approved these changes

@mhdawson mhdawson mhdawson approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@cjihrig cjihrig cjihrig approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@gabrielschulhof @nodejs-github-bot @legendecas @mhdawson @MylesBorins @bnoordhuis @cjihrig @BridgeAR @targos