-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: document the connection event for HTTP2 & TLS servers #34531
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@nodejs/build can you check this? |
I've added a Rerun CI-Lite: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/4195/ |
Landed in e5dacc2 |
PR-URL: #34531 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #34531 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #34531 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #34531 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #34531 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
As discussed in #34296, the documentation isn't totally clear on where 'connection' events can be manually emitted, or that doing so is an officially supported use case for those events.
This PR is a quick first step to improve that - taking the text from the same event in the HTTP module (where this is already documented) and including it in the docs for TLS servers, HTTP2 servers & HTTP2 secure servers.
I've simplified the text from the HTTP event to remove details that I'm not 100% sure are true for non-HTTP servers: setTimeout handling, and the specific class of sockets that could ever possibly be emitted here. I don't think either is required for this to be useful, but happy to add those details here too if somebody can confirm they're equally relevant & true.
Checklist