istanbuljs / test-exclude

test for inclusion or exclusion of paths using globs
ISC License
8 stars 12 forks source link

support negated glob for node_modules and always exclude node_modules #17

Closed bcoe closed 7 years ago

bcoe commented 7 years ago

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 https://github.com/istanbuljs/nyc/pull/348

JaKXz commented 7 years ago

doesn't this revert #10 ?

sindresorhus commented 7 years ago

@JaKXz Read the linked discussion: https://github.com/istanbuljs/nyc/pull/348

JaKXz commented 7 years ago

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

sindresorhus commented 7 years ago

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

bcoe commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

Addressed in + test-exclude@3.0.0.

bcoe commented 7 years ago

@sindresorhus @JaKXz this will be released in https://github.com/istanbuljs/nyc/pull/442

sindresorhus commented 7 years ago

@bcoe Awesome. Thanks for fixing :)