-
Notifications
You must be signed in to change notification settings - Fork 633
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
fix(node): vendor readable-stream from esm.sh #2584
Conversation
3cc8ac3
to
8bc6aa5
Compare
901bf22
to
7b9b71a
Compare
7b9b71a
to
8761118
Compare
de88c82
to
c69f914
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Massive effort @cjihrig, thank you so much for looking into this. This looks mostly good to me and I'm waiting eagerly for fixes in readable-stream
that will allow us to remove even more code. Could you add some README file on how to update node/_stream.js
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
npm. To update the code, run the following command: | ||
|
||
``` | ||
deno run -A --unstable node/_tools/vendor_readable_stream.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Great!
Opening as a draft PR to see how the CI goes while I work on a script to automate the vendoring process.