-
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
docs: add clarification about goals of this project #33
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I'm in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor issues to fix, but looks great! I've been somewhat following the discussions and this kind of clarity is excellent and a clear win in the collaborative department :)
README.md
Outdated
## Goals of Project | ||
|
||
A fully functional code coverage solution using only V8's native coverage | ||
features and minimal user-land modules: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being strict here, this sentence does not really tell me what the following list is for. I would add:
...modules, so that we fit these constraints:
or something to that effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad to make this change 👍
README.md
Outdated
|
||
c8 uses | ||
[bleeding edge Node.js features](https://github.com/nodejs/node/pull/22527), | ||
make sure you're running Node.js `>= 10.10.0`. | ||
|
||
## Goals of Project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Goals of this
Project or Goals of the
Project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On it 👍
@profnandaa I'll let you make these couple patches since you've put your hand up; have a couple other tasks on my plate today, but we can land as soon as we've satisfied @JaKXz. |
Looks good to me, thanks for adding this! |
Clarify project goals: I'd like this project to demonstrate a powerful code coverage solution relying primarily on native V8 functionality, and minimal user-land modules -- @bcoe
@demurgos and I have been having long conversations about implementation details surrounding c8, and I think part of the problem was that I've done a poor job of describing the goals I have in mind (they lived only in my head, which isn't useful).
tldr; I'd like this project to demonstrate a powerful code coverage solution relying primarily on native V8 functionality, and minimal user-land modules.
data-transformations are okay, but I don't want to parse JavaScript with Babel, muck with Node's runtime, etc.