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

add openCover script to support open:cover command on windows #421

Merged
merged 5 commits into from
Apr 18, 2017

Conversation

deanbot
Copy link
Contributor

@deanbot deanbot commented Apr 17, 2017

see issue #395

@coveralls
Copy link

coveralls commented Apr 17, 2017

Coverage Status

Coverage remained the same at 95.283% when pulling 0530534 on deanbot:master into 500371e on coryhouse:master.

@RoM4iK
Copy link
Contributor

RoM4iK commented Apr 17, 2017

Why no ./node_modules/.bin/open ./coverage/lcov-report/index.html in package.json?
I think its better to avoid creation of new file in tools unless its necessary.

@deanbot
Copy link
Contributor Author

deanbot commented Apr 17, 2017

Hi @RoM4iK, open isn't in ./node_modules/.bin/. That being said if you know another way to call open (i.e. open('./coverage/lcov-report/index.html') in package.json without an additional script all the better.

@RoM4iK
Copy link
Contributor

RoM4iK commented Apr 17, 2017

Then, we can move to a better opener package, instead of writing wrappers for existing.
We can look at opn, and exactly on opn-cli

@coryhouse
Copy link
Owner

Ah good idea @RoM4iK. I wasn't aware of opn-cli, but it looks perfect. @deanbot - Would you be willing to adjust your PR to use it?

@deanbot
Copy link
Contributor Author

deanbot commented Apr 17, 2017

No problem.

@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Coverage remained the same at 95.283% when pulling 5b1c14b on deanbot:master into 500371e on coryhouse:master.

Copy link
Owner

@coryhouse coryhouse left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@coryhouse coryhouse merged commit ed631c3 into coryhouse:master Apr 18, 2017
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.

None yet

4 participants