n-api: mark version 5 N-APIs as stable by gabrielschulhof · Pull Request #29401 · nodejs/node (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation9 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

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 added the c++

Issues and PRs that require attention from people who are familiar with C++.

label

Sep 2, 2019

Choose a reason for hiding this comment

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

LGTM but you might want to update the documentation for napi_get_date_value() to specify whether it does or doesn't include leap seconds.

Since it's UNIX time and presumably the equivalent of Date.p.valueOf() I'm guessing it doesn't but there's really no way to tell for a casual reader.

@bnoordhuis I'm actually not sure whether it does or does not account for leap seconds. napi_get_date_value() merely calls v8::Date::ValueOf() which is documented as a specialization of v8::Value::NumberValue without mention of its (lack of) accounting for leap seconds.

Rebased. Hoping this'll fix Travis.

Choose a reason for hiding this comment

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

LGTM

gabrielschulhof pushed a commit that referenced this pull request

Sep 5, 2019

PR-URL: #29401 Reviewed-By: Ben Noordhuis info@bnoordhuis.nl Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request

Sep 5, 2019

PR-URL: nodejs#29401 Reviewed-By: Ben Noordhuis info@bnoordhuis.nl Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com

targos pushed a commit that referenced this pull request

Sep 20, 2019

PR-URL: #29401 Reviewed-By: Ben Noordhuis info@bnoordhuis.nl Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com

This was referenced

Sep 25, 2019

This was referenced

Sep 26, 2019

BethGriggs pushed a commit that referenced this pull request

Oct 1, 2019

PR-URL: #29401 Backport-PR-URL: #29458 Reviewed-By: Ben Noordhuis info@bnoordhuis.nl Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com

BethGriggs added a commit that referenced this pull request

Oct 18, 2019

Notable changes:

• deps: upgrade openssl sources to 1.1.1d (Sam Roberts) #29921
• dns: remove dns.promises experimental warning (cjihrig) #26592
• fs: remove experimental warning for fs.promises (Anna Henningsen) #26581
• n-api: mark version 5 N-APIs as stable (Gabriel Schulhof) #29401
• stream: make Symbol.asyncIterator support stable (Matteo Collina) #26989

PR-URL: #29875

BethGriggs added a commit that referenced this pull request

Oct 19, 2019

Notable changes:

• deps: update npm to 6.11.3 (claudiahdz) #29430
• deps: upgrade openssl sources to 1.1.1d (Sam Roberts) #29921
• dns: remove dns.promises experimental warning (cjihrig) #26592
• fs: remove experimental warning for fs.promises (Anna Henningsen) #26581
• n-api: mark version 5 N-APIs as stable (Gabriel Schulhof) #29401
• stream: make Symbol.asyncIterator support stable (Matteo Collina) #26989

PR-URL: #29875

BethGriggs added a commit that referenced this pull request

Oct 21, 2019

Notable changes:

• deps: update npm to 6.11.3 (claudiahdz) #29430
• deps: upgrade openssl sources to 1.1.1d (Sam Roberts) #29921
• dns: remove dns.promises experimental warning (cjihrig) #26592
• fs: remove experimental warning for fs.promises (Anna Henningsen) #26581
• n-api: mark version 5 N-APIs as stable (Gabriel Schulhof) #29401
• stream: make Symbol.asyncIterator support stable (Matteo Collina) #26989

PR-URL: #29875

BethGriggs added a commit that referenced this pull request

Oct 22, 2019

Notable changes:

• crypto:
  • add support for chacha20-poly1305 for AEAD (chux0519) #24081
  • increase maxmem range from 32 to 53 bits (Tobias Nießen) #28799
• deps:
  • update npm to 6.11.3 (claudiahdz) #29430
  • upgrade openssl sources to 1.1.1d (Sam Roberts) #29921
• dns:
  • remove dns.promises experimental warning (cjihrig) #26592
• fs:
  • remove experimental warning for fs.promises (Anna Henningsen) #26581
• http:
  • makes response.writeHead return the response (Mark S. Everitt) #25974
• http2:
  • makes response.writeHead return the response (Mark S. Everitt) #25974
• n-api:
  • make func argument of napi_create_threadsafe_function optional (legendecas) #27791
  • mark version 5 N-APIs as stable (Gabriel Schulhof) #29401
  • implement date object (Jarrod Connolly) #25917
• process:
  • add --unhandled-rejections flag (Ruben Bridgewater) #26599
• stream:
  • implement Readable.from async iterator utility (Guy Bedford) #27660
  • make Symbol.asyncIterator support stable (Matteo Collina) #26989

PR-URL: #29875

BethGriggs added a commit that referenced this pull request

Oct 22, 2019

Notable changes:

• crypto:
  • add support for chacha20-poly1305 for AEAD (chux0519) #24081
  • increase maxmem range from 32 to 53 bits (Tobias Nießen) #28799
• deps:
  • update npm to 6.11.3 (claudiahdz) #29430
  • upgrade openssl sources to 1.1.1d (Sam Roberts) #29921
• dns:
  • remove dns.promises experimental warning (cjihrig) #26592
• fs:
  • remove experimental warning for fs.promises (Anna Henningsen) #26581
• http:
  • makes response.writeHead return the response (Mark S. Everitt) #25974
• http2:
  • makes response.writeHead return the response (Mark S. Everitt) #25974
• n-api:
  • make func argument of napi_create_threadsafe_function optional (legendecas) #27791
  • mark version 5 N-APIs as stable (Gabriel Schulhof) #29401
  • implement date object (Jarrod Connolly) #25917
• process:
  • add --unhandled-rejections flag (Ruben Bridgewater) #26599
• stream:
  • implement Readable.from async iterator utility (Guy Bedford) #27660
  • make Symbol.asyncIterator support stable (Matteo Collina) #26989

PR-URL: #29875

Reviewers

bnoordhuis bnoordhuis approved these changes

jasnell jasnell approved these changes

cjihrig cjihrig approved these changes

mhdawson mhdawson approved these changes

addaleax Awaiting requested review from addaleax

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.