Event Loop Guide grammar fixes #7479
Conversation
|
Is event loop a proper noun? I don't think it needs to be capitalized. |
|
I have to agree, 'event loop' should probably stay all lowercase. I'm all for the removal of unnecessary whitespace at the end of lines though. |
| any asynchronous I/O or timer and it shuts down cleanly if there are not | ||
| any. | ||
|
|
||
| ## Phases in Detail | ||
|
|
||
| ### timers | ||
| ### `timers`: |
There was a problem hiding this comment.
This probably doesn't need backticks.
|
@nodejs/documentation |
|
|
||
| Looking back at our diagram, any time you call `process.nextTick()` in a | ||
| given phase, all callbacks passed to `process.nextTick()` will be | ||
| resolved before the event loop continues. This can create some bad | ||
| situations because **it allows you to "starve" your I/O by making | ||
| recursive `process.nextTick()` calls.** which prevents the event loop | ||
| recursive `process.nextTick()` calls,** which prevents the event loop |
There was a problem hiding this comment.
Small nit, but the comma probably doesn't need to be inside the **.
|
@cjihrig I swapped back-ticking phases for bolding them, and I think it works a lot better. Take a look and let me know what you think. |
|
I agree backticks should be used strictly for code only. |
|
There are conflicts now. |
# Conflicts: # doc/topics/the-event-loop-timers-and-nexttick.md
|
@cjihrig Got it! Resolved. |
|
|
||
| ### `close callbacks`: | ||
| ### **close callbacks** |
There was a problem hiding this comment.
Is there a need to bold headers?
There was a problem hiding this comment.
@cjihrig Only to follow the pattern of bolding event loop phases. Visually, I think it looked better bolded than not, but that is just me personally.
|
LGTM with a comment. |
|
@cjihrig Ah, nevermind. Looks like the styling doesn't actually get any bolder than bold. Maybe the preview in Atom looked different? Who knows. I went ahead and removed the bolding on headers because it seems to be redundant. |
|
One more ping of @nodejs/documentation. Otherwise, I think this can land. |
|
LGTM. I can see a number of other things I'd like to fix but those can be done separately. |
|
Thanks! Landed in 55250b8. |
PR-URL: #7479 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #7479 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #7479 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #7479 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #7479 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #7479 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
Affected core subsystem(s)
Description of change
Some grammar fixes. Updating formatting consistency like uppercasing 'Event Loop' and codifying headers.