-
-
Notifications
You must be signed in to change notification settings - Fork 359
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: Add support for --exclude-node-modules to subcommands. #1053
feat: Add support for --exclude-node-modules to subcommands. #1053
Conversation
3 similar comments
8678cef
to
b0f29ff
Compare
This adds support for the `--exclude-node-modules` option/setting to instrument, check-coverage and report sub-commands. Add testing to verify that `--exclude-node-modules=false` is honored for all commands.
b0f29ff
to
082bc32
Compare
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.
I know this has been explained to me before, but refresh me on why this couldn't be handled by passing in alternate exclude
rules that don't include node_modules
.
Trust the research you've done on this topic, and am 👍 just curious.
@@ -100,20 +106,7 @@ exports.handler = function (argv) { | |||
? './lib/instrumenters/istanbul' | |||
: './lib/instrumenters/noop' | |||
|
|||
const nyc = new NYC({ |
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.
It's a little weird that this refactor happens in this pull, seems like an improvement to picking and choosing parameters though.
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.
@JaKXz saw that I was manually adding yet another option to this constructor, asked if we could do this.
exclude rules are handled by |
This adds support for the
--exclude-node-modules
option/setting toinstrument, check-coverage and report sub-commands.
Add testing to verify that
--exclude-node-modules=false
is honored forall commands.