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.

Sign up for GitHub

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: updating references to the old node.js file #8092

Closed
wants to merge 2 commits into from
Closed

src: updating references to the old node.js file #8092

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9f4ee5
src: updating references to the old node.js file
danbev Aug 3, 2016
62b4527
removing full path to bootstrap_node.js
danbev Aug 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3346,7 +3346,6 @@ void LoadEnvironment(Environment* env) {
env->isolate()->SetFatalErrorHandler(node::OnFatalError);
env->isolate()->AddMessageListener(OnMessage);

// The node.js file returns a function 'f'
atexit(AtExit);

TryCatch try_catch(env->isolate());
Expand All @@ -3366,16 +3365,17 @@ void LoadEnvironment(Environment* env) {
ReportException(env, try_catch);
exit(10);
}
// The bootstrap_node.js file returns a function 'f'
CHECK(f_value->IsFunction());
Local<Function> f = Local<Function>::Cast(f_value);

// Now we call 'f' with the 'process' variable that we've built up with
// all our bindings. Inside node.js we'll take care of assigning things to
// their places.
// all our bindings. Inside bootstrap_node.js we'll take care of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "internal/bootstrap_node.js".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I might have misunderstood @mscdex comment about the full path as I removed it. I'm happy to change this, but at this point I'm not exactly sure if I should add internal to all references or not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessary to put the full path

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

// assigning things to their places.

// We start the process this way in order to be more modular. Developers
// who do not like how 'src/node.js' setups the module system but do like
// Node's I/O bindings may want to replace 'f' with their own function.
// who do not like how bootstrap_node.js setups the module system but do
// like Node's I/O bindings may want to replace 'f' with their own function.

// Add a reference to the global object
Local<Object> global = env->context()->Global();
Expand Down