Skip to content

Conversation

@johenry
Copy link
Contributor

@johenry johenry commented Oct 6, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@mscdex mscdex added the stream Issues and PRs related to the stream subsystem. label Oct 6, 2017
process.on('exit', function() {
assert.strictEqual(cnt, wclosed, 'writable streams closed');
assert.strictEqual(cnt, rclosed, 'readable streams closed');
wclosed = rclosed = 0;
Copy link
Member

@BridgeAR BridgeAR Oct 6, 2017

Choose a reason for hiding this comment

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

I am not sure why this was added here. In general I think it would be best to just remove the error message in the strictEqual

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, @johenry if you could push a commit that just makes it

assert.strictEqual(cnt, wclosed)
assert.strictEqual(cnt, rclosed)

that'd be great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gibfahn Done. Thanks for the help.

@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 7, 2017
@joyeecheung
Copy link
Member

@joyeecheung
Copy link
Member

joyeecheung commented Oct 10, 2017

Landed in c7d2913 , thanks for the contribution!

joyeecheung pushed a commit that referenced this pull request Oct 10, 2017
PR-URL: #15997
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
PR-URL: #15997
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
PR-URL: nodejs/node#15997
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 17, 2017
PR-URL: #15997
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 18, 2017
PR-URL: #15997
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
PR-URL: #15997
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. stream Issues and PRs related to the stream subsystem. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants