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

feat(config): support istanbul-api instrumentation configuration #73

Merged

Conversation

briandipalma
Copy link
Contributor

Add a configuration option to provide istanbul-api instrumentation configuration

@briandipalma
Copy link
Contributor Author

With the new istanbul-api our symlinked monorepo packages are no longer being added to our coverage reports. Although istanbul-api has now been deprecated so I was tempted to try and move to the newer istanbul packages. This change was quicker and easier though. I could try to upgrade the dependencies in another PR if that's something that would be wanted?

@codecov-io
Copy link

codecov-io commented Jul 15, 2019

Codecov Report

Merging #73 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #73   +/-   ##
======================================
  Coverage    97.7%   97.7%           
======================================
  Files           2       2           
  Lines         131     131           
======================================
  Hits          128     128           
  Misses          3       3
Impacted Files Coverage Δ
src/reporter.js 97.77% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fee4a9...625591d. Read the comment docs.

Copy link
Owner

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

Nice one, thank you for this! 😄 Only one small thing I'd change is to consolidate the config under one top level object. If you wanted to make a followup PR to remove the istanbul-api dependency, that would be awesome! 😄

README.md Outdated Show resolved Hide resolved
src/reporter.js Outdated Show resolved Hide resolved
src/reporter.js Outdated Show resolved Hide resolved
Add a configuration option to provide istanbul-api instrumentation configuration
@briandipalma briandipalma force-pushed the add-instrumentation-configuration branch from 0ae2e67 to 401aee6 Compare July 24, 2019 13:14
@briandipalma
Copy link
Contributor Author

Requested changes all done. Thanks for taking a look at the PR.

Copy link
Owner

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot for your contribution! 😄

@mattlewis92 mattlewis92 merged commit c4f7a9c into mattlewis92:master Jul 24, 2019
@mattlewis92
Copy link
Owner

Released as 2.1.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants