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 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.

10 participants