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

Drop dependency on deprecated gulp-util #54

Merged
merged 1 commit into from
Jan 6, 2018

Conversation

demurgos
Copy link
Contributor

Closes #53

@@ -14,7 +14,7 @@ describe('stream', function () {
it('piping into second plumber should keep piping', function (done) {
gulp.src(fixturesGlob)
.pipe(plumber())
.pipe(gutil.noop())
.pipe(through2.obj())
Copy link

Choose a reason for hiding this comment

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

This is funny because gutil.noop was never a stream, it was just a noop function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest I also was surprised by this: replacing it by an empty function caused the tests to fail. Then I noticed that it's actually calling the function: this functions returns a pass-through object stream. new PassThrough({objectMode: true}) also worked but since through2 was available I used it to match the old behavior:

https://github.com/gulpjs/gulp-util/blob/3c18e3cce5047e591ed353a8486ca1251b7f56c9/lib/noop.js#L4

module.exports = function () {
  return through.obj();
};

If I encounter it in other libraries, I'll submit a PR to document this on gulp-util's README.

Copy link

Choose a reason for hiding this comment

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

Oops, I didn't realize gutil had it as a noop stream. I guess I need to update the readme and blog post.

Copy link

Choose a reason for hiding this comment

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

@demurgos done! thanks for pointing that out.

@phated
Copy link

phated commented Dec 29, 2017

@floatdrop can we get this merged?

@floatdrop floatdrop merged commit c80d68e into floatdrop:master Jan 6, 2018
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