search

grncdr / merge-stream

Merge multiple streams into one interleaved stream
MIT License
214 stars 16 forks source link

Upgrade to readable-stream 3 #31

Closed kyrylkov closed 5 years ago

kyrylkov commented 6 years ago

Upgrade to readable-stream 3 for Node.js 10 for await functionality

stevemao commented 6 years ago

PR welcome @kyrylkov

dpilafian commented 5 years ago

With the current supported versions of node, it looks like the readable-stream dependency can be avoided altogether.

In package.json, deleted the line:

"readable-stream": "^2.0.1"

In index.js, replaced:

var PassThrough = require('readable-stream/passthrough')

with:

const { PassThrough } = require('stream');

https://nodejs.org/api/stream.html#stream_class_stream_passthrough

Resulting test output:

$ npm install
npm notice created a lockfile as package-lock.json. You should commit this file.
added 56 packages from 142 contributors and audited 71 packages in 5.177s
found 0 vulnerabilities

$ npm test
> merge-stream@1.0.1 test /Users/dem/projects/merge-stream
> istanbul cover test.js && istanbul check-cover --statements 100 --branches 100

=============================================================================
Writing coverage object [/Users/dem/Heap/Temp/x/merge-stream/coverage/coverage.json]
Writing coverage reports at [/Users/dem/Heap/Temp/x/merge-stream/coverage]
=============================================================================

=============================== Coverage summary ===============================
Statements   : 100% ( 97/97 )
Branches     : 100% ( 14/14 )
Functions    : 100% ( 29/29 )
Lines        : 100% ( 86/86 )
================================================================================

@stevemao Is this the desired approach?

stevemao commented 5 years ago

@dpilafian Sounds good :)

ehmicky commented 5 years ago

This issue is fixed I think