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

report: move process mutation into bootstrap/node.js #25821

Closed
wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Jan 30, 2019

Please use https://github.com/nodejs/node/pull/25821/files?&w=1 for the ease of review.

This patch moves the part in the report initialization
that mutates the process object into bootstrap/node.js
so it's easier to tell the side effect of the initialization
on the global state during bootstrap.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the process Issues and PRs related to the process subsystem. label Jan 30, 2019
@joyeecheung
Copy link
Member Author

This patch moves the part in the report initialization
that mutates the process object into bootstrap/node.js
so it's easier to tell the side effect of the initialization
on the global state during bootstrap.
@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 31, 2019
@joyeecheung
Copy link
Member Author

Landed in 406329d

@joyeecheung joyeecheung closed this Feb 1, 2019
joyeecheung added a commit that referenced this pull request Feb 1, 2019
This patch moves the part in the report initialization
that mutates the process object into bootstrap/node.js
so it's easier to tell the side effect of the initialization
on the global state during bootstrap.

PR-URL: #25821
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@addaleax
Copy link
Member

addaleax commented Feb 1, 2019

@joyeecheung This also needs to be backported to v11.x manually

targos pushed a commit that referenced this pull request Feb 10, 2019
This patch moves the part in the report initialization
that mutates the process object into bootstrap/node.js
so it's easier to tell the side effect of the initialization
on the global state during bootstrap.

PR-URL: #25821
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos targos mentioned this pull request Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants