-
Notifications
You must be signed in to change notification settings - Fork 459
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
Add ArrayBuffer::Detach() and ::IsDetached() #659
Conversation
8d7c515
to
bcd03dd
Compare
Note that |
@tniessen do you need any more info to progress this? |
@tniessen are you still planning to work on this? |
Yes, sorry @mhdawson, I do intend to work on this. I didn't find much time recently. |
@tniessen np, just wanted to check in as I was doing some cleanup on the issues. |
There was a problem hiding this 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.
There was a problem hiding this 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 🙂
@tniessen ping. |
bcd03dd
to
ad9f5b0
Compare
Sorry it took me so long to get back to this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Looks good landing |
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>
Landed as c9563ca |
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>
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>
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>
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>
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>
I don't know what the proper guard is, seems like
NAPI_EXPERIMENTAL
andNAPI_VERSION
are not good enough.Refs: nodejs/node#29768
Refs: nodejs/node#30613