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

support negated glob for node_modules and always exclude node_modules #17

Closed
bcoe opened this issue Nov 7, 2016 · 9 comments
Closed

Comments

@bcoe
Copy link
Member

bcoe commented Nov 7, 2016

It's surprising behavior that when you set a custom exclude rule you almost always need to add node_modules. We should instead using a negated glob if folks want to instrument node_modules.

See the discussion with @sindresorhus in istanbuljs/nyc#348

@JaKXz
Copy link
Member

JaKXz commented Nov 7, 2016

doesn't this revert #10 ?

@sindresorhus
Copy link
Member

@JaKXz Read the linked discussion: istanbuljs/nyc#348

@JaKXz
Copy link
Member

JaKXz commented Nov 7, 2016

@sindresorhus I did, but the solution is unclear to me. Anyway, I'm not blocking this issue in any way, was just curious.

@sindresorhus
Copy link
Member

@JaKXz How is it unclear? Put ["!node_modules"] in the ignore to enable node_modules again.

@bcoe
Copy link
Member Author

bcoe commented Nov 7, 2016

@JaKXz this will be a breaking change 👍 but, I'm convinced by my conversation with @sindresorhus (and by frequent node_modules related issues opened on nyc, that it's a better call for us to always exclude node_modules, as long as we have a work around we can point people to).

@JaKXz
Copy link
Member

JaKXz commented Nov 7, 2016

@sindresorhus the "unclear"ness was because I was confused about if we're attempting to support both at the same time [which is slightly impossible 😆 ] - when we were implementing #10 I/we should have remembered how globs work and simply provided that documentation. Thanks for raising the issue.

@bcoe
Copy link
Member Author

bcoe commented Nov 13, 2016

Addressed in + test-exclude@3.0.0.

@bcoe bcoe closed this as completed Nov 13, 2016
@bcoe
Copy link
Member Author

bcoe commented Nov 13, 2016

@sindresorhus @JaKXz this will be released in istanbuljs/nyc#442

@sindresorhus
Copy link
Member

@bcoe Awesome. Thanks for fixing :)

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

No branches or pull requests

3 participants