src: reorganize process object creation, property implementation and events in C++ land #25397
Conversation
This file contains 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
nodejs-github-bot
added
c++
joyeecheung
force-pushed
the
move-process
branch
from
112f3cc to
653543d
Compare
jasnell
approved these changes
Jan 8, 2019
addaleax
approved these changes
Jan 8, 2019
| }; | ||
| MakeCallback(env->isolate(), process_object, emit_fn.As<Function>(), | ||
| arraysize(argv), argv, {0, 0}); | ||
| ProcessEmit(env, "internalMessage", message); |
There was a problem hiding this comment.
This sounds like something we might want to migrate off process.emit(), right?
There was a problem hiding this comment.
@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 ..
|
Fixed the includes: https://ci.nodejs.org/job/node-test-pull-request/20022/ |
joyeecheung
added
the
author ready
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
force-pushed
the
move-process
branch
from
5d98be1 to
b05ae4e
Compare
|
Need a rebase to resolve the linter failure. CI: https://ci.nodejs.org/job/node-test-pull-request/20026/ |
|
Landed in 4c9ea8f...84f0581 |
joyeecheung
added a commit
that referenced
this pull request
Jan 11, 2019
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>
joyeecheung
added a commit
that referenced
this pull request
Jan 11, 2019
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>
joyeecheung
added a commit
that referenced
this pull request
Jan 11, 2019
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>
addaleax
pushed a commit
that referenced
this pull request
Jan 15, 2019
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>
addaleax
pushed a commit
that referenced
this pull request
Jan 15, 2019
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>
addaleax
pushed a commit
that referenced
this pull request
Jan 15, 2019
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>
Merged
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 16, 2019
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>
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 16, 2019
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>
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 16, 2019
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>
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 17, 2019
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>
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 17, 2019
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>
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 17, 2019
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>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
src: move process object creation into node_process_object.cc
Changes
SetupProcessObjecttoCreateProessObjectwhich createsthe process object from scratch and return it to
Environment::Startto 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.emitandprocess.emitWarningmethodsfrom
node.ccinto intonode_process_events.cc, andreuse the implementation in other places that need to do
process.emitin C++.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes