Skip to content
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

Revert https://github.com/nodejs/node/pull/5776 #5947

Merged
merged 2 commits into from
Mar 29, 2016
Merged

Conversation

evanlucas
Copy link
Contributor

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

stream, win

Description of change

Revert #5776

See #5927

@evanlucas evanlucas changed the title Rev Revert https://github.com/nodejs/node/pull/5776 Mar 29, 2016
@evanlucas
Copy link
Contributor Author

/cc @mcollina @mafintosh @orangemocha

@mscdex mscdex added stream Issues and PRs related to the stream subsystem. tty Issues and PRs related to the tty subsystem. labels Mar 29, 2016
@mcollina
Copy link
Member

LGTM, it would break things again on windows, but we need to assess the situation better.

cc @piranna

@mcollina
Copy link
Member

@egoroof I didn't imply otherwise, sorry if I was misunderstood.

@orangemocha
Copy link
Contributor

LGTM

@evanlucas
Copy link
Contributor Author

@cjihrig
Copy link
Contributor

cjihrig commented Mar 29, 2016

LGTM

This reverts commit ace1009.

The offending commit broke certain usages of piping from stdin.

Fixes: nodejs#5927
PR-URL: nodejs#5947
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit 4611389.

The offending commit broke certain usages of piping from stdin.

Fixes: nodejs#5927
PR-URL: nodejs#5947
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@evanlucas
Copy link
Contributor Author

Landed in 89abe86 and b6475b9. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem. tty Issues and PRs related to the tty subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants