FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: clean up program/isolate/env init logic by bnoordhuis · Pull Request #9224 · nodejs/node · GitHub

/ node Public

src: clean up program/isolate/env init logic - #9224

Merged
bnoordhuis merged 2 commits into
nodejs:masterfrom
bnoordhuis:simplify-startup
Oct 24, 2016
ConversationCommits (2)ChecksFiles changed
Merged

src: clean up program/isolate/env init logic#9224
bnoordhuis merged 2 commits into
nodejs:masterfrom
bnoordhuis:simplify-startup

Conversation

bnoordhuis commented Oct 21, 2016

Copy link
Copy Markdown
Member

General cleanup:

  1. Shuffle around the startup sequence so it's less a ball of twine, and
  2. Remove the unnecessary NodeInstance class.

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 21, 2016
jasnell approved these changes Oct 21, 2016
View reviewed changes

jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with green CI

bnoordhuis commented Oct 22, 2016

Copy link
Copy Markdown
Member Author

I started the CI yesterday but it doesn't show up. New run: https://ci.nodejs.org/job/node-test-pull-request/4618/

jasnell commented Oct 22, 2016

Copy link
Copy Markdown
Member

Some red there but they appear to be unrelated.

src: simplify code, remove NodeInstanceData
d77e818 
NodeInstanceData is not used meaningfully and makes the initialization
logic harder to follow.  Let's remove it and delete 100 lines of code
in one fell swoop.

PR-URL: nodejs#9224
Reviewed-By: James M Snell <jasnell@gmail.com>
src: clean up program/isolate/env init logic
ceb6023 
Reorder the initialization logic so that program-wide, per-isolate and
per-environment initialization is more cleanly separated.

PR-URL: nodejs#9224
Reviewed-By: James M Snell <jasnell@gmail.com>
bnoordhuis closed this Oct 24, 2016
bnoordhuis deleted the simplify-startup branch October 24, 2016 20:42
bnoordhuis merged commit ceb6023 into nodejs:master Oct 24, 2016
evanlucas pushed a commit that referenced this pull request Nov 2, 2016
src: simplify code, remove NodeInstanceData
9e753ba 
NodeInstanceData is not used meaningfully and makes the initialization
logic harder to follow.  Let's remove it and delete 100 lines of code
in one fell swoop.

PR-URL: #9224
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request Nov 2, 2016
src: clean up program/isolate/env init logic
f2a3b24 
Reorder the initialization logic so that program-wide, per-isolate and
per-environment initialization is more cleanly separated.

PR-URL: #9224
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas mentioned this pull request Nov 3, 2016
Merged

MylesBorins commented Nov 17, 2016

Copy link
Copy Markdown
Contributor

@bnoordhuis should this be backported? I'm assuming it shouldn't land on v4. It does not land cleanly on v6. Please feel free to update labels

bnoordhuis commented Nov 18, 2016

Copy link
Copy Markdown
Member Author

Back-porting isn't necessary unless it causes conflicts for other pull requests. Let me know if that is the case.

MylesBorins added the baking-for-lts PRs that need to wait before landing in a LTS release. label May 8, 2017
MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL