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

readable-stream version #2

Closed
Klowner opened this issue Dec 22, 2015 · 8 comments
Closed

readable-stream version #2

Klowner opened this issue Dec 22, 2015 · 8 comments

Comments

@Klowner
Copy link

Klowner commented Dec 22, 2015

Hi, just curious if you'd be open to changing dependency to any version of readable-stream and bump to 1.0? This library does a fantastic job of fixing an issue in vinyl-fs and would definitely like to incorporate it.

Cheers!

-Mark

@phated
Copy link

phated commented Dec 22, 2015

To clarify, this module attempts to use the built in stream module before attempting to use readable-stream but I think it would be a better idea to always you readable-stream directly (not any version, just latest). See https://r.va.gg/2014/06/why-i-dont-use-nodes-core-stream-module.html

@phated
Copy link

phated commented Dec 22, 2015

The above issues are being brought up because we'd like to use this module in the next version of gulp.

@jpommerening
Copy link
Owner

Sounds like a pretty good idea. I noticed the ancient version a few weeks ago but didn't bother to do anything about it ("If it ain't broke…").

Oh, and sorry about the delay. Family time ;)

Looking back at the conditional require('readable-stream') I absolutely agree that it's not a good idea now that we have 3 different major streams APIs.

jpommerening added a commit that referenced this issue Dec 29, 2015
@jpommerening
Copy link
Owner

@Klowner can you try lazystream@1.0.0-rc1 and tell me if that'll do?

@Klowner
Copy link
Author

Klowner commented Dec 29, 2015

@jpommerening works perfectly here! Everything look good to you, @phated ?

@phated
Copy link

phated commented Dec 29, 2015

Yep. LGTM

@jpommerening
Copy link
Owner

Ok, published as 1.0.0. Thanks, guys!

@Klowner
Copy link
Author

Klowner commented Dec 29, 2015

Thank you! 👍

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

3 participants