Skip to content

Conversation

@addaleax
Copy link
Member

Ref: #13452

Checklist
Affected core subsystem(s)

doc/async_hooks

@nodejs-github-botnodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. doc Issues and PRs related to the documentations. labels Jun 8, 2017
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it was there before but probably should to call -> to be called
Less sure but maybe correspond -> correlate
Correspond: have a close similarity; match or agree almost exactly.
Correlate: have a mutual relationship or connection, in which one thing affects or depends on another.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it was there before but probably should to call -> to be called

Ah right, done.

Correspond: have a close similarity; match or agree almost exactly.

Well, that’s basically exactly what it’s supposed to say, so it seems like choosing correspond is perfect. :)

@trevnorris
Copy link
Contributor

Don't agree with this changed for same reasons stated in #13452 (comment). Users should know what they can filter for from node core, and should also be able to find the same in documentation from module authors that create their own types.

@addaleax
Copy link
MemberAuthor

@nodejs/async_hooks … I don’t have terribly strong feelings either way.

@Fishrock123
Copy link
Contributor

Why not update this rather than remove it? 😕

@refack
Copy link
Contributor

Should we just expose process.binding('async_wrap').Providers?

@addaleaxaddaleaxforce-pushed the async-hooks-doc-remove-type-list branch from e9178fe to a9eb903CompareJune 8, 2017 22:40
@addaleaxaddaleax changed the title doc: remove async_hooks providers listdoc: update async_hooks providers listJun 8, 2017
@addaleax
Copy link
MemberAuthor

@trevnorris@Fishrock123 Seems like I misread the discussion in #13452 a bit. I’ve updated this to add Timeout and Immediate instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't Timeout and Immediate be uppercase or no?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, these values are emitted from JS and are written this way in the source code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth adding a note to point that out ... specifically, the all-caps ones come from native code while the mixed case come from JavaScript

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget TickObject! 😄

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trevnorris Added. :)

@addaleaxaddaleaxforce-pushed the async-hooks-doc-remove-type-list branch from a9eb903 to 4bbad95CompareJune 10, 2017 10:11
@trevnorris
Copy link
Contributor

@refack

Should we just expose process.binding('async_wrap').Providers?

I'm down for that, but please expose it as types (or Types) instead. "Providers" is for legacy reasons and I never liked that name.

@refackrefack mentioned this pull request Jun 11, 2017
4 tasks
@refack
Copy link
Contributor

I'm down for that, but please expose it as types (or Types) instead. "Providers" is for legacy reasons and I never liked that name.

Ref: #13610

jasnell pushed a commit that referenced this pull request Jun 13, 2017
PR-URL: #13561 Ref: #13452 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

Landed in 0ab4614

@jasnelljasnell closed this Jun 13, 2017
addaleax added a commit that referenced this pull request Jun 17, 2017
PR-URL: #13561 Ref: #13452 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]>
@addaleaxaddaleax mentioned this pull request Jun 17, 2017
addaleax added a commit that referenced this pull request Jun 21, 2017
PR-URL: #13561 Ref: #13452 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]>
@addaleaxaddaleax mentioned this pull request Jun 21, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_hooksIssues and PRs related to the async hooks subsystem.docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@addaleax@trevnorris@Fishrock123@refack@jasnell@mscdex@MylesBorins@nodejs-github-bot