-
Notifications
You must be signed in to change notification settings - Fork 465
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
Implement CallbackScope class #362
Conversation
If someone has a permission, please add |
@romandev thanks for picking this up :) |
cdca219
to
d60a593
Compare
d60a593
to
4adf148
Compare
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 just few suggestion for documentation.
doc/callback_scope.md
Outdated
``` | ||
|
||
- `[in] env`: The environment in which to create the `Napi::CallbackScope`. | ||
- `[in] scope`: pre-existing `napi_callback_scope` or `Napi::CallbackScope`. |
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.
pre-existing -> The pre-existing
doc/callback_scope.md
Outdated
``` | ||
|
||
- `[in] env`: The environment in which to create the `Napi::CallbackScope`. | ||
- `[in] async_context`: pre-existing `napi_async_context` or `Napi::AsyncContext`. |
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.
pre-existing -> The pre-existing
This is a wrapper class to support the following N-APIs. - napi_open_callback_scope() - napi_close_callback_scope() Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope
4adf148
to
579e865
Compare
Addressed your comments. Thank you for review! |
Could you please remove |
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. @romandev thanks for adding this.
This is a wrapper class to support the following N-APIs. - napi_open_callback_scope() - napi_close_callback_scope() Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope PR-URL: #362 Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
Landed as 405f3e5 |
This is a wrapper class to support the following N-APIs. - napi_open_callback_scope() - napi_close_callback_scope() Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope PR-URL: nodejs/node-addon-api#362 Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
This is a wrapper class to support the following N-APIs. - napi_open_callback_scope() - napi_close_callback_scope() Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope PR-URL: nodejs/node-addon-api#362 Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
This is a wrapper class to support the following N-APIs. - napi_open_callback_scope() - napi_close_callback_scope() Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope PR-URL: nodejs/node-addon-api#362 Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
This is a wrapper class to support the following N-APIs. - napi_open_callback_scope() - napi_close_callback_scope() Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope PR-URL: nodejs/node-addon-api#362 Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
This is a wrapper class to support the following N-APIs.
Refs: https://nodejs.org/api/n-api.html#n_api_napi_open_callback_scope