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

Fix#422 - Installed karma-coverage and istanbul dependencies #464

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
2 participants
@MarkKrutik
Copy link

commented Sep 23, 2018

This was an addon that will allow to add coverage to the code. It is not fully implemented though. If you want any changes to be done, please let me know.

MarkKrutik added some commits Sep 21, 2018

@humphd
Copy link
Contributor

left a comment

I'm happy to see this happening. A few things need to happen to move this forward:

  1. We need to pull the coverage/ directory out of this PR. To do that: git rm -fr coverage
  2. Add coverage/ to the .gitignore file so that it won't commit these files by accident.
  3. Remove the instanbul dep from package.json, which I don't think we need. I could be wrong, but I think that karma-coverage will pull it in automatically.
  4. The test you changed in tests/spec/fs.write.spec.js should probably go in another file or PR. Was this something else you were working on? Best to keep things on their own branches.

After that, let me know and we can do another round on this.

@@ -1,18 +1,31 @@
module.exports = function(config) {
config.set({
npm_config_coverage: ['true'],

This comment has been minimized.

Copy link
@humphd

humphd Oct 9, 2018

Contributor

I'm not familiar with this, what does it do?

@humphd

This comment has been minimized.

Copy link
Contributor

commented Nov 18, 2018

@MarkKrutik thanks for starting this. I spent some time this week trying to get coverage to work with Karma and Parcel, and in the end, I don't think it's possible. I've got another solution based on https://github.com/istanbuljs/nyc, which I'll do a PR for in a sec and CC you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.