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

remove gulp-shell #790

Closed
yocontra opened this issue Jul 7, 2015 · 7 comments
Closed

remove gulp-shell #790

yocontra opened this issue Jul 7, 2015 · 7 comments

Comments

@yocontra
Copy link

yocontra commented Jul 7, 2015

https://github.com/google/material-design-lite/blob/master/gulpfile.js#L601 should be replaced with node's built-in child_process module. gulp.src('').pipe(plugin()) is a really nasty pattern that emerged when people started making plugins for things that shouldn't be plugins. gulp.src('') will start to error in the final 4.0 release to discourage people from doing this

@yocontra
Copy link
Author

yocontra commented Jul 7, 2015

As a sidenote, gulp-shell is useful but only when you need to use template strings that are based on the file-name and commands that run per-file. I'll ask them to add that to the README

@addyosmani
Copy link
Contributor

It's entirely my fault for letting this pattern land in reviews. Agree we should be just using child_process here. We'll get this fixed up. Thanks for catching this, @contra!

@addyosmani addyosmani added the P1 label Jul 7, 2015
@yocontra
Copy link
Author

yocontra commented Jul 7, 2015

No problem, just wanted to make sure the google gulpfiles are in good shape because people use them for reference all the time. I can review some of the other ones when I catch a spare minute and send some PRs as well.

@surma
Copy link
Contributor

surma commented Jul 7, 2015

Any feedback regarding the gulpfile is welcome, @contra! Thank you. I plan to do a complete overhaul because it’s become quite organic and convoluted (inefficient, even).

@addyosmani
Copy link
Contributor

I can review some of the other ones when I catch a spare minute and send some PRs as well.

👍 ✨

@yocontra
Copy link
Author

yocontra commented Jul 7, 2015

@surma If you have any questions feel free to ping me on gulp issues, IRC, or twitter whenever

@traviskaufman
Copy link
Contributor

Closing this as we are moving away from gulp in v2 and there hasn't been much activity on this issue in the past year. If someone would like to contribute the change to the mdl-1.x branch we can reopen it.

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

4 participants