FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

process: make internal/queue_microtask.js more self-contained by joyeecheung · Pull Request #25189 · nodejs/node · GitHub

/ node Public

process: make internal/queue_microtask.js more self-contained - #25189

Closed
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:queue-microtask
ConversationCommits (1)ChecksFiles changed
Closed

process: make internal/queue_microtask.js more self-contained#25189
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:queue-microtask

Conversation

joyeecheung commented Dec 23, 2018

Copy link
Copy Markdown
Member
  • Instead of passing triggerFatalException through node.js,
    simply put it on internalBinding('util') which has to be
    loaded anyway.
  • Expose the implementation of queueMicrotask directly instead
    of through an unnecessary factory function.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

joyeecheung requested a review from devsnek December 23, 2018 02:30

nodejs-github-bot commented Dec 23, 2018

Copy link
Copy Markdown
Collaborator

@joyeecheung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2045/pipeline

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Dec 23, 2018

joyeecheung commented Dec 23, 2018

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/19756/

Trott reviewed Dec 23, 2018
View reviewed changes
Comment thread lib/internal/bootstrap/node.js Outdated
Trott reviewed Dec 23, 2018
View reviewed changes
Comment thread lib/internal/queue_microtask.js Outdated
process: make internal/queue_microtask.js more self-contained
cfb7811 
- Instead of passing triggerFatalException through node.js,
  simply put it on `internalBinding('util')` which has to be
  loaded anyway.
- Expose the implementation of `queueMicrotask` directly instead
  of through an unnecessary factory function.

joyeecheung commented Dec 24, 2018

Copy link
Copy Markdown
Member Author

Rebased and fixed linter errors: https://ci.nodejs.org/job/node-test-pull-request/19773/

Trott commented Dec 24, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/19784/ ✔️

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 24, 2018

joyeecheung commented Dec 29, 2018

Copy link
Copy Markdown
Member Author

Landed in 8a60be4

joyeecheung added a commit that referenced this pull request Dec 29, 2018
process: make internal/queue_microtask.js more self-contained
8a60be4 
- Instead of passing triggerFatalException through node.js,
  simply put it on `internalBinding('util')` which has to be
  loaded anyway.
- Expose the implementation of `queueMicrotask` directly instead
  of through an unnecessary factory function.

PR-URL: #25189
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

addaleax commented Jan 5, 2019

Copy link
Copy Markdown
Member

Should this be backported to v11.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
process: make internal/queue_microtask.js more self-contained
eafd8a6 
- Instead of passing triggerFatalException through node.js,
  simply put it on `internalBinding('util')` which has to be
  loaded anyway.
- Expose the implementation of `queueMicrotask` directly instead
  of through an unnecessary factory function.

PR-URL: nodejs#25189
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

addaleax commented Jan 15, 2019

Copy link
Copy Markdown
Member

This lands cleanly now.

addaleax pushed a commit that referenced this pull request Jan 15, 2019
process: make internal/queue_microtask.js more self-contained
7be4a39 
- Instead of passing triggerFatalException through node.js,
  simply put it on `internalBinding('util')` which has to be
  loaded anyway.
- Expose the implementation of `queueMicrotask` directly instead
  of through an unnecessary factory function.

PR-URL: #25189
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR mentioned this pull request Jan 16, 2019
Merged
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
process: make internal/queue_microtask.js more self-contained
59997e0 
- Instead of passing triggerFatalException through node.js,
  simply put it on `internalBinding('util')` which has to be
  loaded anyway.
- Expose the implementation of `queueMicrotask` directly instead
  of through an unnecessary factory function.

PR-URL: nodejs#25189
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 17, 2019
process: make internal/queue_microtask.js more self-contained
f32e6a8 
- Instead of passing triggerFatalException through node.js,
  simply put it on `internalBinding('util')` which has to be
  loaded anyway.
- Expose the implementation of `queueMicrotask` directly instead
  of through an unnecessary factory function.

PR-URL: nodejs#25189
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins mentioned this pull request Jan 24, 2019
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Trott Trott left review comments
jasnell jasnell approved these changes
BridgeAR BridgeAR approved these changes
+1 more reviewer
devsnek devsnek approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL