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.

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

Removing subprocess. #499

Closed
wants to merge 1 commit into from

Conversation

stelcheck
Copy link
Contributor

@stelcheck stelcheck commented Dec 20, 2017

This commit:

  1. Cleans up how the code is loaded
  2. Removes the necessity to use 'spawn'

Replaces #476
Replaces #489

Fixes #459
Fixes #471
Fixes #480
Fixes #487
Fixes #498

(phew - hope I don't forget anyone!)

This commit:

  1. Cleans up how the code is loaded
  2. Removes the necessity to use 'spawn'

Replaces TypeStrong#476
Replaces TypeStrong#489
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 76.75% when pulling b7d4312 on stelcheck:chores/remove-spawn into 129153f on TypeStrong:master.

1 similar comment
@coveralls
Copy link

coveralls commented Dec 20, 2017

Coverage Status

Coverage decreased (-0.7%) to 76.75% when pulling b7d4312 on stelcheck:chores/remove-spawn into 129153f on TypeStrong:master.

@stelcheck
Copy link
Contributor Author

One question: I expect that the way I extract flags from minimist that there could be collisions between userland flags and ts-node flags.

@blakeembrey
Copy link
Member

@stelcheck I think I'm missing something here. How are you using the node.js flags?

@blakeembrey
Copy link
Member

For reference, it seems like this is undoing #131?

@stelcheck
Copy link
Contributor Author

Right... I was thinking they would get applied, but you are right.

I guess we have two options:

  1. Revert to a wrapper not written in Node
  2. Direct users to use NODE_OPTIONS

The first option would be tricky to implement properly I think, so I am trying to implement tjis without a wrapper. What is you take on it?

@blakeembrey
Copy link
Member

Thanks for your patience. Finally got around to this and closing with #536.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants