-
Notifications
You must be signed in to change notification settings - Fork 91
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: Node API #19
feat: Node API #19
Conversation
8b6dff4
to
06f3629
Compare
This commit attempts to refactor the code to separate the running/reporting/writing logic so that it can be consumed by other libraries
06f3629
to
f854b7d
Compare
😄 |
I had some time to look at your proposal. From what I see, your current API proposal is intended to be executed at the client level. It has a few issues:
Providing helpers for reporters is fine. There's probably some bike-shedding if it should return V8 reports and let the consumer convert them to Istanbul's format or not. (So I'd start with a more conservative API with similar capabilities as the CLI (so the CLI is just a wrapper of the Node API). The instrumented code would still have to be executed in its own sub-process. What I would provide is a function with an API similar to |
@demurgos @laggingreflex 👋 sorry for not having helped steward this project in recent months. This project started out mainly as a proof of concept, to prove out whether or not we could potentially use v8's native test coverage for Node.js projects. Well things work relatively well, I ultimately landed on the opinion that we'd do better to try to integrate things more tightly into Node.js itself. I'm hoping to continue working with @hashseed @schuay to better define what this would look like. If you're interested in joining this conversation, or want to bounce any ideas off of me related to c8, I can be found in chat here: |
@laggingreflex see #22 |
This commit attempts to refactor the code to separate the running/reporting/writing logic so that it can be consumed by other libraries