/node

n-api: take n-api out of experimental #19262

Closed
wants to merge 2 commits into
from
+3 −88

Conversation

Reviewers

@richardlau richardlau

@mcollina mcollina

@fhinkel fhinkel

@jasnell jasnell

@targos targos

@cjihrig cjihrig

@devsnek devsnek

Assignees
No one assigned
Labels
Projects
None yet
Milestone
  10.0.0
9 participants
@mhdawson @mcollina @fhinkel @jasnell @targos @cjihrig @richardlau @devsnek @nodejs-github-bot
@mhdawson
Owner

mhdawson commented Mar 9, 2018

Take n-api out of experimental as per:
nodejs/TSC#501

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

@nodejs-github-bot nodejs-github-bot added C++ lib / src labels Mar 9, 2018

@mhdawson mhdawson referenced this pull request in nodejs/TSC Mar 9, 2018

Closed

Bring N-API out of experimental #501

@mcollina

mcollina approved these changes Mar 9, 2018
View changes

LGTM

src/node.cc
- return;
- }
- }
+ // N-API module this is ok

This comment has been minimized.

Show comment Hide comment
@cjihrig

cjihrig Mar 9, 2018

Contributor

I think it would be better to combine the mp->nm_version == -1 check with the one in the else if

@cjihrig

cjihrig Mar 9, 2018

Contributor

I think it would be better to combine the mp->nm_version == -1 check with the one in the else if

@jasnell jasnell added this to the 10.0.0 milestone Mar 9, 2018

@mhdawson

This comment has been minimized.

Show comment Hide comment
@mhdawson

mhdawson Mar 9, 2018

Owner

@cjihrig pushed commit to address your comment.

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

Owner

mhdawson commented Mar 9, 2018

@cjihrig pushed commit to address your comment.

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

@mhdawson mhdawson added the author ready label Mar 9, 2018

@richardlau richardlau added the notable-change label Mar 10, 2018

doc/api/n-api.md
@@ -2,7 +2,7 @@
<!--introduced_in=v7.10.0-->
-> Stability: 1 - Experimental
+> Stability: 2 - Stable

This comment has been minimized.

Show comment Hide comment
@richardlau

richardlau Mar 10, 2018

Member

Do we track changes like this in the documentation with YAML metadata?

@richardlau

richardlau Mar 10, 2018

Member

Do we track changes like this in the documentation with YAML metadata?

This comment has been minimized.

Show comment Hide comment
@mhdawson

mhdawson Mar 14, 2018

Owner

Looking at the existing docs, I don't think so.

@mhdawson

mhdawson Mar 14, 2018

Owner

Looking at the existing docs, I don't think so.

mhdawson added some commits Mar 9, 2018

@mhdawson
n-api: take n-api out of experimental 
Take n-api out of experimental as per:
nodejs/TSC#501
f28dfe5
@mhdawson
squash: address comments

Some checks were not successful

1 failing and 16 successful checks
@nodejs-github-bot
node-test-commit — tests failed
Details
@nodejs-github-bot
node-test-commit-aix — tests passed
Details
@nodejs-github-bot
node-test-commit-arm-fanned — tests passed
Details
@nodejs-github-bot
node-test-commit-freebsd — tests passed
Details
@nodejs-github-bot
node-test-commit-linux — tests passed
Details
@nodejs-github-bot
node-test-commit-linuxone — tests passed
Details
@nodejs-github-bot
node-test-commit-osx — tests passed
Details
@nodejs-github-bot
node-test-commit-plinux — tests passed
Details
@nodejs-github-bot
node-test-commit-smartos — tests passed
Details
@nodejs-github-bot
node-test-commit-windows-fanned — flaky tests failed
Details
@nodejs-github-bot
node-test-linter — tests passed
Details
@nodejs-github-bot
node-test-linux-linked-debug — tests passed
Details
@nodejs-github-bot
node-test-linux-linked-fips20 — tests passed
Details
@nodejs-github-bot
node-test-linux-linked-openssl102 — tests passed
Details
@nodejs-github-bot
node-test-linux-linked-openssl110 — tests passed
Details
@nodejs-github-bot
node-test-linux-linked-withoutintl — tests passed
Details
@nodejs-github-bot
node-test-linux-linked-zlib — tests passed
Details
8220f5c
@mhdawson

This comment has been minimized.

Show comment Hide comment
@mhdawson

mhdawson Mar 14, 2018

Owner

rebased, new ci

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

Owner

mhdawson commented Mar 14, 2018

rebased, new ci

ci: https://ci.nodejs.org/job/node-test-pull-request/13673/
@mhdawson

This comment has been minimized.

Show comment Hide comment
@mhdawson

mhdawson Mar 14, 2018

Owner

Faiure on linux was known issue: #18657. Rest of CI is green so we are good to go, will land.

Owner

mhdawson commented Mar 14, 2018

Faiure on linux was known issue: #18657. Rest of CI is green so we are good to go, will land.

@mhdawson mhdawson added the semver-minor label Mar 14, 2018

@mhdawson

This comment has been minimized.

Show comment Hide comment
@mhdawson

mhdawson Mar 14, 2018

Owner

land as cd7d7b1

Owner

mhdawson commented Mar 14, 2018

land as cd7d7b1

@mhdawson mhdawson closed this Mar 14, 2018

mhdawson added a commit that referenced this pull request Mar 14, 2018

@mhdawson
n-api: take n-api out of experimental 
Take n-api out of experimental as per:
nodejs/TSC#501

PR-URL: #19262
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
cd7d7b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment