Skip to content
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

Merged
merged 5 commits into from
Oct 17, 2015
Merged

docs: add up to date documentation for AsyncWrap #27

merged 5 commits into from
Oct 17, 2015

Conversation

AndreasMadsen
Copy link
Member

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.

@pmuellr
Copy link
Contributor

pmuellr commented Oct 5, 2015

@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.

@trevnorris
Copy link

@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.

@AndreasMadsen
Copy link
Member Author

@trevnorris Thanks for the review. I have updated the documentation based on your comments and nodejs/node#3216 (add parent object).

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.

This comment was marked as off-topic.

@AndreasMadsen
Copy link
Member Author

Thanks @Qard, I have updated the documentation.

@pmuellr
Copy link
Contributor

pmuellr commented Oct 13, 2015

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.

@trevnorris
Copy link

finished another round. mainly grammer and stuff. great stuff.

@AndreasMadsen
Copy link
Member Author

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.

@trevnorris
Copy link

Two nits. Other than that I would say LGTM. Nice bit of writing you did!

@AndreasMadsen
Copy link
Member Author

Fixed.

@AndreasMadsen
Copy link
Member Author

@Qard @pmuellr Any last comments?

@Qard
Copy link
Member

Qard commented Oct 16, 2015

LGTM

AndreasMadsen added a commit that referenced this pull request Oct 17, 2015
docs: add up to date documentation for AsyncWrap
@AndreasMadsen AndreasMadsen merged commit 4a67d03 into nodejs:master Oct 17, 2015
@AndreasMadsen AndreasMadsen mentioned this pull request Dec 28, 2015
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants