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

spawn istanbul in such a way that works for [email protected] #33

Merged
merged 4 commits into from
Jul 25, 2015

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Jul 10, 2015

@rmg pointed out in this pull request #30 that istanbul was being invoked in a manner that would work poorly if istanbul got hoisted up into a higher folder.

I've switched to using an approach which @othiym23 and @iarna suggested today:

  1. I aded istanbul to npm run scripts.
  2. I muck with the cwd, so that I can invoke the npm run script from the context of nyc.
  3. I invoke the npm run script.

@jasisk I also added the statements option in this pull we didn't actually collide on the s key, because s was used on the outer yargs chain, not the inner yargs chain.

Let me know what ya'll think.

@rmg
Copy link
Contributor

rmg commented Jul 10, 2015

I was thinking more like just replacing the resolve with something like this:

require.resolve('istanbul/lib/cli');

@bcoe
Copy link
Member Author

bcoe commented Jul 10, 2015

@rmg that's clever, I like that it wouldn't require calling chdir.

@jasisk
Copy link
Contributor

jasisk commented Jul 10, 2015

👍. Sorry for the oversight on my part.

bcoe added a commit that referenced this pull request Jul 25, 2015
spawn istanbul in such a way that works for [email protected]
@bcoe bcoe merged commit 9efc5fa into master Jul 25, 2015
@bcoe bcoe deleted the safe-spawn branch July 25, 2015 20:46
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