Skip to content

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

@trevnorris can you review?

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 comment was marked as off-topic.

This comment was marked as off-topic.

@AndreasMadsen
Copy link
MemberAuthor

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

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@AndreasMadsen
Copy link
MemberAuthor

Thanks @Qard, I have updated the documentation.

@pmuellr
Copy link
Contributor

I assume we're ready to merge this? Or are more updates coming?

This comment was marked as off-topic.

@trevnorris
Copy link

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

@AndreasMadsen
Copy link
MemberAuthor

Thanks @trevnorris.

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
MemberAuthor

Fixed.

@AndreasMadsen
Copy link
MemberAuthor

@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
@AndreasMadsenAndreasMadsen merged commit 4a67d03 into nodejs:masterOct 17, 2015
@AndreasMadsenAndreasMadsen mentioned this pull request Dec 28, 2015
27 tasks
Sign up for freeto 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

@AndreasMadsen@pmuellr@trevnorris@Qard@watson