Skip to content
This repository has been archived by the owner on May 10, 2020. It is now read-only.

[DO NOT MERGE] chore: nyc --all #4

Closed
wants to merge 4 commits into from
Closed

[DO NOT MERGE] chore: nyc --all #4

wants to merge 4 commits into from

Conversation

JaKXz
Copy link

@JaKXz JaKXz commented Jul 8, 2016

@iam4x just wanted to make this PR so we can discuss your project-specific (non-nyc) things. I have to say I like your setup and I'm excited to see if we can get webpack and nyc to play nice :)

cc @bcoe

@JaKXz JaKXz changed the title chore: nyc --all [DO NOT MERGE] chore: nyc --all Jul 8, 2016
JaKXz added 2 commits July 8, 2016 19:11
The file specified is not available and making webpack complain, so commenting I'm commenting it out for now
@codecov-io
Copy link

Current coverage is 100%

Merging #4 into master will not change coverage

@@           master    #4   diff @@
===================================
  Files           5     3     -2   
  Lines          38    12    -26   
  Methods         0     0          
  Messages        0     0          
  Branches        0     0          
===================================
- Hits           38    12    -26   
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last updated by e51bd10...0624507

@iam4x
Copy link
Owner

iam4x commented Jul 13, 2016

So much thank's for the work @JaKXz it works great 👍

I think we can merge this into master? It will need a little rebase but seems good :)

@JaKXz
Copy link
Author

JaKXz commented Jul 13, 2016

No worries @iam4x! I wouldn't pull the trigger on merging just yet because I think the wrong files are being covered.

This was meant to be more of a demonstration of the right config / what would be expected to work. My other hypothesis is that the babel-webpack-loaders plugin is conflicting with nyc and I haven't had a chance to look closely at how that plugin works (cc @bcoe).

Perhaps you can try making all the same changes with the latest versions of nyc and babel-plugin-istanbul and see if things go better? If possible, I'd also suggest using a file structure and naming for tests that plays a little nicer with AVA and nyc. There's a good argument for putting your tests alongside the code you're writing that @kentcdodds put in a nice article. The bonus there is that AVA looks for those .test.js files and nyc excludes them from coverage automatically.

@kentcdodds
Copy link

There's a good argument for putting your tests alongside the code you're writing that @kentcdodds put in a nice article.

Thanks for the shoutout @JaKXz. Read that here: http://kcd.im/co-location

The bonus there is that AVA looks for those .test.js files and nyc excludes them from coverage automatically.

Yeah, I have opinions about things and I make pull requests to libraries so my opinions integrate well 😉

@bcoe
Copy link

bcoe commented Jul 14, 2016

@JaKXz @iam4x I will happily land any pull requests ASAP, that help move us closer to supporting webpack appropriately...

an aside, It's getting to the point that there are enough different: configurations, frameworks, dials, knobs, and testing frameworks that interact with nyc/istanbuljs that I think we should consider getting documentation website built (something like http://yargs.js.org/) that outlines some common configurations ... the README.md is getting a bit byzantine. what do you think @gotwarlost?

@gotwarlost
Copy link

@bcoe sounds good to me.

@JaKXz JaKXz closed this Aug 14, 2016
@JaKXz JaKXz deleted the chore/nyc-all branch August 14, 2016 19:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants