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

doc,test: mention Duplex support for TLS #17599

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Document and test the existing support for generic Duplex streams in the TLS module.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

tls

Document and test the existing support for generic Duplex streams
in the TLS module.
@addaleax addaleax added doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem. labels Dec 11, 2017
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Dec 11, 2017
assert.strictEqual(serverTLS.read().toString(), 'foobar');
assert.strictEqual(clientTLS._handle.writeQueueSize, 0);
}));
assert.ok(clientTLS._handle.writeQueueSize > 0);
Copy link
Member Author

Choose a reason for hiding this comment

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

@apapirovski I can remove these checks if you think they’ll conflict with your work?

Just seemed like a nice check to verify that #17564 doesn’t change this behavior :)

Copy link
Member

Choose a reason for hiding this comment

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

This should work just fine, I believe. Will confirm as soon as I've got something actually working... :)

@addaleax addaleax removed the tools Issues and PRs related to the tools directory. label Dec 11, 2017
@addaleax
Copy link
Member Author

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 11, 2017
@BridgeAR
Copy link
Member

Landed in 93656f4

@BridgeAR BridgeAR closed this Dec 12, 2017
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Dec 12, 2017
Document and test the existing support for generic Duplex streams
in the TLS module.

PR-URL: nodejs#17599
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 13, 2017
MylesBorins pushed a commit that referenced this pull request Dec 20, 2017
Document and test the existing support for generic Duplex streams
in the TLS module.

PR-URL: #17599
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2017
Document and test the existing support for generic Duplex streams
in the TLS module.

PR-URL: #17599
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
@MylesBorins
Copy link
Contributor

Is this applicable to v6 or v8?

@addaleax addaleax deleted the tls-generic-stream branch January 22, 2018 14:35
@addaleax
Copy link
Member Author

@MylesBorins Yup, this can safely land in both.

MylesBorins pushed a commit that referenced this pull request Feb 27, 2018
Document and test the existing support for generic Duplex streams
in the TLS module.

PR-URL: #17599
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 27, 2018
Document and test the existing support for generic Duplex streams
in the TLS module.

PR-URL: #17599
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants