Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
src: add node_process.cc#21105
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
src: add node_process.cc #21105
Uh oh!
There was an error while loading. Please reload this page.
Conversation
jasnell commented Jun 3, 2018
devsnek left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rubber stamp lgtm
refack left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍
addaleax commented Jun 3, 2018
LGTM, but this will need a non-trivial rebase against #20876 and quick backports against other release lines ¯\_(ツ)_/¯ |
jasnell commented Jun 3, 2018
addaleax commented Jun 3, 2018
@jasnell It’s labelled |
jasnell commented Jun 3, 2018
Awesome. 🎉 |
trivikr left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM
addaleax commented Jun 7, 2018
@jasnell This is ready for rebasing now :) |
jasnell commented Jun 7, 2018
Awesome. Will try to rebase tomorrow |
Begin moving `process` object function definitions out of `node.cc` ... continuing the process of making `node.cc` smaller and easier to maintain.
39bfa5f to c87f004Comparejasnell commented Jun 10, 2018
@addaleax ... rebased! PTAL New CI: https://ci.nodejs.org/job/node-test-pull-request/15375/ |
jasnell commented Jun 10, 2018
Some related failures in CI... trying again: https://ci.nodejs.org/job/node-test-pull-request/15377/ |
jasnell commented Jun 11, 2018
One flaky failure in Linux... re-running https://ci.nodejs.org/job/node-test-commit-linux/19468/ |
Trott commented Jun 11, 2018
Trott commented Jun 11, 2018
And again: https://ci.nodejs.org/job/node-test-pull-request/15387/ (Related: Consider approving #21251.) |
Begin moving `process` object function definitions out of `node.cc` ... continuing the process of making `node.cc` smaller and easier to maintain. PR-URL: #21105 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
jasnell commented Jun 14, 2018
Landed in baadc7a |
jasnell commented Jun 14, 2018
(note... landed this initially without metadata, caught it, then did a quick force push with the metadata... how? you ask, I forgot to |
joyeecheung commented Jun 15, 2018
@jasnell Have you tried adding Maybe nodejs/node-core-utils#160 could help with that as well |
targos commented Jun 15, 2018
@jasnell would you be able to backport this to v10.x-staging asap? It probably has to be done from scratch instead of a cherry-pick to avoid sneaking in semver-major changes. |
addaleax commented Jun 15, 2018
@targos Are you still planning on merging the Worker changes into v10.x in the near future? I think they might have a tiny conflict with one of the patches in the security release – but a big one with this PR (i.e. I’d recommend doing this after the Worker PR – if there are nontrivial merge conflicts, I’m happy to help) |
targos commented Jun 15, 2018
I've already merged everything from master apart from this PR, including de Worker changes |
jasnell commented Jun 15, 2018
Will do a backport PR early next week. |
targos commented Jun 22, 2018
Ping @jasnell |
targos commented Jul 3, 2018
Ping. Does anyone want to pick this up? |
targos commented Jul 10, 2018
Ping. Commits that depend on this change are accumulating. |
jasnell commented Jul 10, 2018
I'll be able to get back on backporting this week. |
targos commented Jul 10, 2018
Super. Thank you! |
Backport of nodejs#21105
Begin moving `process` object function definitions out of `node.cc` ... continuing the process of making `node.cc` smaller and easier to maintain. PR-URL: nodejs#21105 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Begin moving `process` object function definitions out of `node.cc` ... continuing the process of making `node.cc` smaller and easier to maintain. PR-URL: #21105 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Backport-PR-URL: #21799 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Begin moving `process` object function definitions out of `node.cc` ... continuing the process of making `node.cc` smaller and easier to maintain. Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Backport-PR-URL: #21798 PR-URL: #21105
Begin moving `process` object function definitions out of `node.cc` ... continuing the process of making `node.cc` smaller and easier to maintain. Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Backport-PR-URL: #21798 PR-URL: #21105
Begin moving
processobject function definitions out ofnode.cc... continuing the process of makingnode.ccsmaller and easier to maintain.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes