-
Notifications
You must be signed in to change notification settings - Fork 110
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
Remove deprecated istanbul-api package. #263
Conversation
@kategengler / @rwwagner90 - Would one of y'all mind testing this against your applications to confirm things are working appropriately? |
@rwjblue happy to help test, but what exactly am I looking for? Just that coverage matches previous versions? I'm trying to identify where the gap in our tests here is and what would be different checking manually in an app. Just want to make sure I am looking for the right things! |
@rwwagner90 See #264 We discovered the tests weren't running just after this PR. |
@rwjblue I just tried this out and got |
@rwwagner90 See #265 |
@rwjblue @kategengler let me know when I should test this stuff out then. Happy to help when it's ready! |
a86b955
to
d929e7f
Compare
I've rebased the commit and resolved the conflicts. CI is green now, so I'm assuming this is good to go? |
@Turbo87 looks like there are some more conflicts. Once resolved, I would assume we could go ahead and merge. |
istanbul-api was deprecated in favor of using the individual `instanbul-*` libraries migrated to here.
Rebased again. |
d929e7f
to
a4998ce
Compare
istanbul-api
was deprecated in favor of using the individualinstanbul-*
libraries migrated to here.Because we don't currently support running code coverage against a dummy app's code, and this addon doesn't have any code in addon/addon-test-support/app I can't actually verify if this is working very easily.
The things that I would like verified are:
COVERAGE=true ember test
emits a valid coverage file, and the reports look correct