Skip to content

Use own "isStream" helper#2427

Closed
zertosh wants to merge 1 commit intorequest:masterfrom
zertosh:remove-isstream
Closed

Use own "isStream" helper#2427
zertosh wants to merge 1 commit intorequest:masterfrom
zertosh:remove-isstream

Conversation

@zertosh
Copy link
Copy Markdown
Contributor

@zertosh zertosh commented Oct 16, 2016

Equivalent check as isstream, but this way related functionality is kept together for clarity (next to the preexisting isReadStream) - and you simplify the dependency graph a bit.

@mikeal
Copy link
Copy Markdown
Member

mikeal commented Oct 17, 2016

Is that helper in the stream module pre-v6? v4 tests are failing.

@zertosh
Copy link
Copy Markdown
Contributor Author

zertosh commented Oct 17, 2016

Fixed, but the 6.8.1 failure seems fishy given nodejs/node#9088. In any case, that failure would be present regardless of this change since the instanceof check is exactly the same in isstream.

@FredKSchott
Copy link
Copy Markdown
Contributor

This PR has been closed as "abandoned" as a part of an automated cleanup. If this is a mistake and you are still interested in merging this PR, please do the following:

  1. Rebase your changes onto the master branch (even if there are no merge conflicts, we want to make sure your change is under the most recent tests).
  2. Push your updated branch to Github to update this PR.
  3. Make sure this PR's Travis tests are still passing. (Run npm test locally if tests are still failing).
  4. Comment here to let us know that you've updated the PR and are still interested in getting it merged. Somebody from the project will respond and work with you to review.

Thank you for your understanding. If you have any questions, please check out #2556 and feel free to comment / ask anything there.

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