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

Anti-gulp API #71

Closed
phated opened this issue Sep 21, 2015 · 2 comments
Closed

Anti-gulp API #71

phated opened this issue Sep 21, 2015 · 2 comments

Comments

@phated
Copy link

phated commented Sep 21, 2015

This module breaks many of the gulp plugin contracts and leads people to create poorly structured tasks.

Nesting a stream inside an on('finish') is an antipattern and giant red flag that things are being done incorrectly. Why is the hookRequire method even part of a stream?

Streams should be returned from the gulp task. By nesting the stream inside the event callback, you are unable to return that stream pipeline. You can use the callback as per the example but you lose a bunch of internal stuff that gulp does, like sink the stream, etc.

@SBoudrias
Copy link
Owner

Hey, I understand you though this would be helpful, but if you think we're doing it wrong, then please suggest the gulp workflow you'd like to see.

We're open to changes, but you need to let us know what it is you envision.

@phated
Copy link
Author

phated commented Sep 21, 2015

Having a single pipeline instead of 2 separate ones (maybe using gulp-if or gulp-filter) would go a long way. Updating examples to return the streams would too. I don't have availability to QA all modules and usually just throw them on the blacklist when they are causing multiple issues to be brought up on the main gulp repo.

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

No branches or pull requests

2 participants