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

Throw on invalid glob, clean up, 100% coverage, closes #34 #35

Merged
merged 1 commit into from
Jan 2, 2015

Conversation

UltCombo
Copy link
Contributor

@UltCombo UltCombo commented Jan 2, 2015

@coveralls
Copy link

Coverage Status

Coverage increased (+1.49%) when pulling 989d162 on UltCombo:clean-up into 2768041 on wearefractal:master.

@UltCombo
Copy link
Contributor Author

UltCombo commented Jan 2, 2015

Shiny 100% 😺

return false;
if (pattern instanceof RegExp) return true;
if (typeof pattern === 'string') return pattern[0] === '!';
throw new Error('Invalid glob argument');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should emit the glob index that was invalid. I think this validation belongs at the top of the main globs loop not in a sub-function

@UltCombo
Copy link
Contributor Author

UltCombo commented Jan 2, 2015

Thinking again, the matcher idea is pretty pointless now that node-glob is about to land the ignore API, I'll revert that part.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.49%) when pulling cf60019 on UltCombo:clean-up into 2768041 on wearefractal:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.49%) when pulling 4e4be75 on UltCombo:clean-up into 2768041 on wearefractal:master.

- Better naming for negative matchers ( gulpjs#33 (comment) )
- Throw on invalid glob argument ( gulpjs#34 (comment) )
- Removed the `if (!Array.isArray(globs)) return gs.createStream(globs, [], opt);` because this optimization is already covered a couple lines below at `if (positives.length === 1) return streamFromPositive(positives[0]);`, and to not duplicate the validation code. Previously, passing a string with a negative glob did not throw a "Missing positive glob" error, now it does.
@coveralls
Copy link

Coverage Status

Coverage increased (+1.49%) when pulling 17381e7 on UltCombo:clean-up into 2768041 on wearefractal:master.

@UltCombo
Copy link
Contributor Author

UltCombo commented Jan 2, 2015

PTAL.

@yocontra
Copy link
Member

yocontra commented Jan 2, 2015

Nice and clean now - thanks!

yocontra pushed a commit that referenced this pull request Jan 2, 2015
Throw on invalid glob, clean up, 100% coverage, closes #34
@yocontra yocontra merged commit 58358ff into gulpjs:master Jan 2, 2015
@UltCombo UltCombo deleted the clean-up branch January 2, 2015 23:46
phated pushed a commit that referenced this pull request Feb 16, 2017
Throw on invalid glob, clean up, 100% coverage, closes #34
phated pushed a commit that referenced this pull request Feb 16, 2017
Throw on invalid glob, clean up, 100% coverage, closes #34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants