doc, http2: add sections for server.close() and note to clarify behavior#19802
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
nodejs-github-bot added doc 
addaleax reviewed Apr 4, 2018
doc/api/http2.md Outdated
Member
There was a problem hiding this comment.
Can you make sure that these versions are right? I think it’s 8.4.0, the version that the whole http2 module was originally introduced in
ContributorAuthor
There was a problem hiding this comment.
Yes, that makes sense. Will fix.
doc/api/http2.md Outdated
Contributor
There was a problem hiding this comment.
A nit: it seems this need to go before [`tls.TLSSocket`]:, these references are sorted in ASCII order.
ContributorAuthor
There was a problem hiding this comment.
Whoops, yep I can sort.
vsemozhetbyt approved these changes Apr 4, 2018
Contributor
vsemozhetbyt left a comment
vsemozhetbyt left a comment There was a problem hiding this comment.
Doc format LGTM, with @addaleax's comment addressed)
Clarify current behavior of http2server.close() and http2secureServer.close() w.r.t. perceived differences when compared with httpServer.close(). Fixes: nodejs#19711
ContributorAuthor
chrismilleruk commented Apr 4, 2018
Review comments addressed and PR updated. |
vsemozhetbyt added the fast-track 
Contributor
vsemozhetbyt commented Apr 4, 2018
vsemozhetbyt added the author ready 
trivikr approved these changes Apr 4, 2018
Member
trivikr left a comment
trivikr left a comment There was a problem hiding this comment.
LGTM, verified at private branch
cc @nodejs/http2
trivikr pushed a commit that referenced this pull request Apr 7, 2018
Clarify current behavior of http2server.close() and http2secureServer.close() w.r.t. perceived differences when compared with httpServer.close(). Fixes: #19711 PR-URL: #19802 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Member
trivikr commented Apr 7, 2018
Landed in c60c93c Congrats @chrismilleruk for your first commit in Node.js core! 🎉🎉🎉 |
targos pushed a commit that referenced this pull request Apr 12, 2018
Clarify current behavior of http2server.close() and http2secureServer.close() w.r.t. perceived differences when compared with httpServer.close(). Fixes: #19711 PR-URL: #19802 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
kjin pushed a commit to kjin/node that referenced this pull request May 1, 2018
Clarify current behavior of http2server.close() and http2secureServer.close() w.r.t. perceived differences when compared with httpServer.close(). Fixes: nodejs#19711 PR-URL: nodejs#19802 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 2, 2018
Clarify current behavior of http2server.close() and http2secureServer.close() w.r.t. perceived differences when compared with httpServer.close(). Fixes: #19711 Backport-PR-URL: #20456 PR-URL: #19802 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Merged
MylesBorins pushed a commit that referenced this pull request May 15, 2018
Clarify current behavior of http2server.close() and http2secureServer.close() w.r.t. perceived differences when compared with httpServer.close(). Fixes: #19711 Backport-PR-URL: #20456 PR-URL: #19802 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 15, 2018
Clarify current behavior of http2server.close() and http2secureServer.close() w.r.t. perceived differences when compared with httpServer.close(). Fixes: #19711 Backport-PR-URL: #20456 PR-URL: #19802 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clarify current behavior of https2server.close() and http2secureServer.close()
w.r.t. perceived differences when compared with httpServer.close().
Fixes: #19711
I'm open to discussion on whether this is the right approach / wording. Thanks in advance for any help :-)
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes