Skip to content

Add ArrayBuffer::Detach() and ::IsDetached() #659

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
Closed
wants to merge 2 commits into from
Closed

Add ArrayBuffer::Detach() and ::IsDetached() #659

wants to merge 2 commits into from

Conversation

@tniessen
Copy link
Member

@tniessen tniessen commented Jan 26, 2020 •
edited
Loading

I don't know what the proper guard is, seems like NAPI_EXPERIMENTAL and NAPI_VERSION are not good enough.

Refs: nodejs/node#29768
Refs: nodejs/node#30613

@tniessen tniessen force-pushed the add-detach-functions branch from 8d7c515 to bcd03dd Compare January 26, 2020 05:37
@legendecas
Copy link
Member

legendecas commented Jan 26, 2020 •
edited
Loading

NODE_MAJOR_VERSION can be used in tests to guards on Node.js major version so that no-back-ported Node.js release can work on CI.

Note that NODE_MAJOR_VERSION shall only be used in tests but not in napi.h and napi-inl.h.

@mhdawson
Copy link
Member

mhdawson commented Mar 18, 2020

@tniessen do you need any more info to progress this?

@mhdawson
Copy link
Member

mhdawson commented Jun 9, 2020

@tniessen are you still planning to work on this?

@tniessen
Copy link
Member Author

tniessen commented Jun 10, 2020

Yes, sorry @mhdawson, I do intend to work on this. I didn't find much time recently.

@mhdawson
Copy link
Member

mhdawson commented Jun 11, 2020

@tniessen np, just wanted to check in as I was doing some cleanup on the issues.

gabrielschulhof
gabrielschulhof approved these changes Aug 6, 2020
View reviewed changes
Copy link
Contributor

@gabrielschulhof gabrielschulhof left a comment

Choose a reason for hiding this comment

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

This needs a rebase, but otherwise LGTM.

gabrielschulhof
gabrielschulhof reviewed Aug 6, 2020
View reviewed changes
Copy link
Contributor

@gabrielschulhof gabrielschulhof left a comment

Choose a reason for hiding this comment

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

Oh, wait, and documentation 🙂

@mhdawson
Copy link
Member

mhdawson commented Sep 22, 2020

@tniessen ping.

@tniessen tniessen force-pushed the add-detach-functions branch from bcd03dd to ad9f5b0 Compare November 5, 2020 14:12
@tniessen tniessen marked this pull request as ready for review November 5, 2020 14:18
@tniessen
Copy link
Member Author

tniessen commented Nov 5, 2020

Sorry it took me so long to get back to this!

@mhdawson
Copy link
Member

mhdawson commented Nov 5, 2020

CI run on 10.x as oldest LTS: https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/3022/

CI run on head: https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/3025/

mhdawson
mhdawson approved these changes Nov 5, 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.

LGTM

@mhdawson
Copy link
Member

mhdawson commented Nov 5, 2020

Looks good landing

mhdawson pushed a commit that referenced this pull request Nov 5, 2020
@tniessen @mhdawson
src: add ArrayBuffer::Detach() and ::IsDetached()
c9563ca 
Refs: nodejs/node#29768
Refs: nodejs/node#30613

PR-URL: #659
Refs: nodejs/node#29768
Refs: nodejs/node#30613
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
@mhdawson
Copy link
Member

mhdawson commented Nov 5, 2020

Landed as c9563ca

@mhdawson mhdawson closed this Nov 5, 2020
@tniessen tniessen deleted the add-detach-functions branch November 5, 2020 21:23
Superlokkus pushed a commit to Superlokkus/node-addon-api that referenced this pull request Nov 20, 2020
@tniessen @Superlokkus
src: add ArrayBuffer::Detach() and ::IsDetached()
586e584 
Refs: nodejs/node#29768
Refs: nodejs/node#30613

PR-URL: nodejs#659
Refs: nodejs/node#29768
Refs: nodejs/node#30613
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
@kevindavies8
src: add ArrayBuffer::Detach() and ::IsDetached()
ae16785 
Refs: nodejs/node#29768
Refs: nodejs/node#30613

PR-URL: nodejs/node-addon-api#659
Refs: nodejs/node#29768
Refs: nodejs/node#30613
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
@Marlyfleitas
src: add ArrayBuffer::Detach() and ::IsDetached()
e538d3c 
Refs: nodejs/node#29768
Refs: nodejs/node#30613

PR-URL: nodejs/node-addon-api#659
Refs: nodejs/node#29768
Refs: nodejs/node#30613
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
@wroy7860
src: add ArrayBuffer::Detach() and ::IsDetached()
f0588bc 
Refs: nodejs/node#29768
Refs: nodejs/node#30613

PR-URL: nodejs/node-addon-api#659
Refs: nodejs/node#29768
Refs: nodejs/node#30613
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
src: add ArrayBuffer::Detach() and ::IsDetached()
39f355a 
Refs: nodejs/node#29768
Refs: nodejs/node#30613

PR-URL: nodejs/node-addon-api#659
Refs: nodejs/node#29768
Refs: nodejs/node#30613
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gabrielschulhof gabrielschulhof gabrielschulhof approved these changes

@mhdawson mhdawson mhdawson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tniessen @legendecas @mhdawson @gabrielschulhof