Skip to content

Update readable-stream#2

Merged
jhiesey merged 2 commits into
jhiesey:masterfrom
jimmywarting:master
Sep 25, 2018
Merged

Update readable-stream#2
jhiesey merged 2 commits into
jhiesey:masterfrom
jimmywarting:master

Conversation

@jimmywarting
Copy link
Copy Markdown
Collaborator

@jimmywarting jimmywarting commented Sep 5, 2018

Hi, I don't directly use range-slice-stream but one of your dependency is outdated (readable-stream v2)

I have another package that uses readable-stream v3 and your v2 is making lots of duplicated code

I updated your code to use es6 that is compatible with latest node LTS ( node v6 )
therefore i removed inherits and replaced it with class extends

And now you can't call RangeSliceStream() without new And readable-stream made a major update so i thought a new major version where in place here to

@jhiesey
Copy link
Copy Markdown
Owner

jhiesey commented Sep 25, 2018

Thanks, looks good!

@jhiesey jhiesey merged commit b40a6f3 into jhiesey:master Sep 25, 2018
@jhiesey
Copy link
Copy Markdown
Owner

jhiesey commented Sep 25, 2018

In the future I'd prefer PRs didn't include version number changes.

@jimmywarting
Copy link
Copy Markdown
Collaborator Author

jimmywarting commented Sep 25, 2018

In the future I'd prefer PRs didn't include version number changes.

Got it

Comment thread package.json
"dependencies": {
"inherits": "^2.0.1",
"readable-stream": "^2.0.5"
"readable-stream": "^3.0.2"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@jhiesey I think this could create "a little mess" within videostream.js etc, I.e. readable-stream should ideally also get upgraded on mediasource & mp4-stream (?)

$ npm ls readable-stream
videostream@2.6.0
+-- mediasource@2.2.2
| `-- readable-stream@2.3.6
+-- mp4-stream@2.0.3
| `-- readable-stream@2.3.6
+-- range-slice-stream@2.0.0
| `-- readable-stream@3.0.3
`-- readable-stream@3.0.3

Copy link
Copy Markdown
Collaborator Author

@jimmywarting jimmywarting Sep 28, 2018

Choose a reason for hiding this comment

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

I know
I'm trying to help all package involved in webtorrent to try and update readable-stream to v3

https://npm.anvaka.com/#/view/2d/webtorrent

feross/mediasource#18
mafintosh/mp4-stream#20

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks! 👍

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