-
Notifications
You must be signed in to change notification settings - Fork 78
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
docs: add up to date documentation for AsyncWrap #27
Conversation
@trevnorris can you review? |
... | ||
``` | ||
|
||
_tick_ indicates there is at least one tick between the text above and the text below. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
@AndreasMadsen Thanks for all the text. I think my referenced patch is the last for the near future. There are some grammatical things, but we'll worry about those later. |
this. | ||
|
||
* Timer functions (like `setTimeout`) shares a single Timer handle, thus you | ||
will usually have to money patch those functions. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
@trevnorris Thanks for the review. I have updated the documentation based on your comments and nodejs/node#3216 (add |
and `init` hook has two extra arguments `provider` and `parent`. | ||
|
||
```javascript | ||
function init(provider, parent) { this = handle; } |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Thanks @Qard, I have updated the documentation. |
I assume we're ready to merge this? Or are more updates coming? |
Node.js. | ||
AsyncWrap is two things. One is a | ||
[class abstraction](https://github.com/nodejs/node/blob/master/src/async-wrap.h) | ||
there provides node internal facilities for handling async things, such as |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
finished another round. mainly grammer and stuff. great stuff. |
Thanks @trevnorris. |
const asyncWrap = process.binding('async_wrap'); | ||
``` | ||
|
||
_Be warned that this API is not an official API and can change at any time, even if it's just patch |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Two nits. Other than that I would say LGTM. Nice bit of writing you did! |
Fixed. |
LGTM |
docs: add up to date documentation for AsyncWrap
This should be an up to date documentation on the AsyncWrap API. I have not included in discussion on the future of the API, but there a small section (Things you might not expect) on some stuff that other users should know.
Also I'm not really sure what the purpose of the provider argument is and how it compares to
this.constructor.name
someone else should elaborate on that.PS: I've heard my spelling isn't ideal, bear with me.