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

doc: clarify sentence in event loop doc #8400

Closed
wants to merge 1 commit into from

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Sep 3, 2016

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change
  • Make a sentence in the event loop doc easier to understand.
  • Fix a couple of nits (trailing spaces and missing periods).

Ref: #8370 (diff)

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Sep 3, 2016
@jasnell
Copy link
Member

jasnell commented Sep 4, 2016

LGTM

1 similar comment
@cjihrig
Copy link
Contributor

cjihrig commented Sep 5, 2016

LGTM

* **check**: `setImmediate()` callbacks are invoked here
* **close callbacks**: e.g socket.on('close', ...)
* **I/O callbacks**: executes almost all callbacks with the exception of
close callbacks, the ones scheduled by timers and `setImmediate()`.
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer a comma before and. That's consistent with the title.

@fhinkel
Copy link
Member

fhinkel commented Sep 6, 2016

LGTM with tiny, tiny comment.

jasnell pushed a commit that referenced this pull request Sep 9, 2016
Refs: #8370 (diff)
PR-URL: #8400
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 9, 2016

Landed in 88ed3d2

@jasnell jasnell closed this Sep 9, 2016
@lpinca lpinca deleted the clarify/sentence branch September 9, 2016 16:40
Fishrock123 pushed a commit that referenced this pull request Sep 14, 2016
Refs: #8370 (diff)
PR-URL: #8400
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants