Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Better pipe cleanup for early shrinkwrap reading #8718

Closed
wants to merge 3 commits into from

Conversation

iarna
Copy link
Contributor

@iarna iarna commented Jun 27, 2015

Fixes #8695

@iarna iarna added the bug label Jun 27, 2015
@iarna iarna added this to the 3.0.1 milestone Jun 27, 2015
stream = untar
addClose()
}

function addClose () {
stream.close = function () {
tounpipe.forEach(dispose)
tounpipe.forEach(function (streams) {
unpipe(streams[0],streams[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

afaict, unpipe only takes one argument, and it unpipes all piped streams. See https://www.npmjs.com/package/unpipe (and source: https://github.com/stream-utils/unpipe/blob/master/index.js#L40)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct! Good catch.

@iarna iarna force-pushed the iarna/fix-packaged-shrinkwrap branch from f864d04 to 60ecd29 Compare July 1, 2015 08:59
iarna added 2 commits July 1, 2015 05:00
for 0.8 compatibility instead of trying to kill the streams by hand
@iarna iarna force-pushed the iarna/fix-packaged-shrinkwrap branch from 60ecd29 to afee6ff Compare July 1, 2015 09:00
iarna added a commit that referenced this pull request Jul 1, 2015
for 0.8 compatibility instead of trying to kill the streams by hand

Fixes #8695

PR-URL: #8718
@iarna
Copy link
Contributor Author

iarna commented Jul 1, 2015

Landed as 57c3cea

@iarna iarna closed this Jul 1, 2015
@iarna iarna removed the in-progress label Jul 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants