src: reorganize process object creation, property implementation and events in C++ land by joyeecheung · Pull Request #25397 · nodejs/node

joyeecheung · 2019-01-08T16:29:26Z

src: move process object creation into node_process_object.cc

Changes SetupProcessObject to CreateProessObject which creates
the process object from scratch and return it to Environment::Start
to be stored in the Environment object.

src: declare process-related C++ methods in node_process.h

Instead of in node_internals.h. Also move process property
accessors that are not reused into node_process_object.cc
and make them static.

process: move C++ process events into node_process_events.cc

Move the C++ process.emit and process.emitWarning methods
from node.cc into into node_process_events.cc, and
reuse the implementation in other places that need to do
process.emit in C++.

Checklist

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

nodejs-github-bot · 2019-01-08T16:29:28Z

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

joyeecheung · 2019-01-08T16:30:39Z

~~CI: https://ci.nodejs.org/job/node-test-pull-request/19999/~~
CI: https://ci.nodejs.org/job/node-test-pull-request/20001/

jasnell

Very happy to see the ongoing cleanup and reorganization here.

src/node.cc

addaleax · 2019-01-08T22:44:12Z

src/inspector_agent.cc

This sounds like something we might want to migrate off process.emit(), right?

@addaleax yeah, this looks like the only place where internalMessage is emitted, maybe the C++ side can call a method passed from JS side somehow instead ..

joyeecheung · 2019-01-09T15:01:59Z

Fixed the includes: https://ci.nodejs.org/job/node-test-pull-request/20022/

Changes `SetupProcessObject` to `CreateProessObject` which creates the process object from scratch and return it to `Environment::Start` to be stored in the Environment object.

Instead of in node_internals.h. Also move process property accessors that are not reused into node_process_object.cc and make them static.

Move the C++ `process.emit` and `process.emitWarning` methods from `node.cc` into into `node_process_events.cc`, and reuse the implementation in other places that need to do `process.emit` in C++.

joyeecheung · 2019-01-09T16:08:43Z

Need a rebase to resolve the linter failure. CI: https://ci.nodejs.org/job/node-test-pull-request/20026/

joyeecheung · 2019-01-11T12:13:27Z

Landed in 4c9ea8f...84f0581

Changes `SetupProcessObject` to `CreateProessObject` which creates the process object from scratch and return it to `Environment::Start` to be stored in the Environment object. PR-URL: #25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Instead of in node_internals.h. Also move process property accessors that are not reused into node_process_object.cc and make them static. PR-URL: #25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Move the C++ `process.emit` and `process.emitWarning` methods from `node.cc` into into `node_process_events.cc`, and reuse the implementation in other places that need to do `process.emit` in C++. PR-URL: #25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Changes `SetupProcessObject` to `CreateProessObject` which creates the process object from scratch and return it to `Environment::Start` to be stored in the Environment object. PR-URL: #25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Instead of in node_internals.h. Also move process property accessors that are not reused into node_process_object.cc and make them static. PR-URL: #25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Move the C++ `process.emit` and `process.emitWarning` methods from `node.cc` into into `node_process_events.cc`, and reuse the implementation in other places that need to do `process.emit` in C++. PR-URL: #25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Changes `SetupProcessObject` to `CreateProessObject` which creates the process object from scratch and return it to `Environment::Start` to be stored in the Environment object. PR-URL: nodejs#25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Instead of in node_internals.h. Also move process property accessors that are not reused into node_process_object.cc and make them static. PR-URL: nodejs#25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Move the C++ `process.emit` and `process.emitWarning` methods from `node.cc` into into `node_process_events.cc`, and reuse the implementation in other places that need to do `process.emit` in C++. PR-URL: nodejs#25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Changes `SetupProcessObject` to `CreateProessObject` which creates the process object from scratch and return it to `Environment::Start` to be stored in the Environment object. PR-URL: nodejs#25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Instead of in node_internals.h. Also move process property accessors that are not reused into node_process_object.cc and make them static. PR-URL: nodejs#25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Move the C++ `process.emit` and `process.emitWarning` methods from `node.cc` into into `node_process_events.cc`, and reuse the implementation in other places that need to do `process.emit` in C++. PR-URL: nodejs#25397 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

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 Jan 8, 2019

joyeecheung force-pushed the move-process branch from 112f3cc to 653543d Compare January 8, 2019 16:36

jasnell approved these changes Jan 8, 2019

View reviewed changes

addaleax approved these changes Jan 8, 2019

View reviewed changes

joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 9, 2019

joyeecheung added 7 commits January 10, 2019 00:04

src: move process object creation into node_process_object.cc

60d3a5e

Changes `SetupProcessObject` to `CreateProessObject` which creates the process object from scratch and return it to `Environment::Start` to be stored in the Environment object.

src: declare process-related C++ methods in node_process.h

a28145b

Instead of in node_internals.h. Also move process property accessors that are not reused into node_process_object.cc and make them static.

process: move C++ process events into node_process_events.cc

5e1b8a2

Move the C++ `process.emit` and `process.emitWarning` methods from `node.cc` into into `node_process_events.cc`, and reuse the implementation in other places that need to do `process.emit` in C++.

fixup! src: move process object creation into node_process_object.cc

ff4e9de

fixup! src: declare process-related C++ methods in node_process.h

cc8d7e3

fixup! src: move process object creation into node_process_object.cc

cdd42e7

fixup! process: move C++ process events into node_process_events.cc

b05ae4e

joyeecheung force-pushed the move-process branch from 5d98be1 to b05ae4e Compare January 9, 2019 16:08

joyeecheung closed this Jan 11, 2019

BridgeAR mentioned this pull request Jan 16, 2019

v11.7.0 proposal #25537

Merged

MylesBorins mentioned this pull request Jan 24, 2019

v11.8.0 proposal #25687

Merged

cola119 mentioned this pull request May 3, 2022

src: remove unnecessary comment in node_process_events.cc #42952

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

src: reorganize process object creation, property implementation and events in C++ land#25397

src: reorganize process object creation, property implementation and events in C++ land#25397
joyeecheung wants to merge 7 commits intonodejs:masterfrom
joyeecheung:move-process

joyeecheung commented Jan 8, 2019

Uh oh!

nodejs-github-bot commented Jan 8, 2019

Uh oh!

joyeecheung commented Jan 8, 2019 •

edited

Loading

Uh oh!

jasnell left a comment

Uh oh!

Uh oh!

addaleax Jan 8, 2019

Uh oh!

joyeecheung Jan 9, 2019

Uh oh!

joyeecheung commented Jan 9, 2019

Uh oh!

joyeecheung commented Jan 9, 2019

Uh oh!

joyeecheung commented Jan 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

joyeecheung commented Jan 8, 2019

src: move process object creation into node_process_object.cc

src: declare process-related C++ methods in node_process.h

process: move C++ process events into node_process_events.cc

Checklist

Uh oh!

nodejs-github-bot commented Jan 8, 2019

Uh oh!

joyeecheung commented Jan 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

addaleax Jan 8, 2019

Choose a reason for hiding this comment

Uh oh!

joyeecheung Jan 9, 2019

Choose a reason for hiding this comment

Uh oh!

joyeecheung commented Jan 9, 2019

Uh oh!

joyeecheung commented Jan 9, 2019

Uh oh!

joyeecheung commented Jan 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joyeecheung commented Jan 8, 2019 •

edited

Loading