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.

Sign up for GitHub

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

Jump to bottom

doc: added explicitly that the close event does not take arguments #20031

Closed
wants to merge 2 commits into from
Closed

doc: added explicitly that the close event does not take arguments #20031

wants to merge 2 commits into from

Conversation

indranil
Copy link
Contributor

@indranil indranil commented Apr 14, 2018 •
edited
Loading

Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: #20018

Checklist

@indranil
doc: added explicitly that the close event does not take arguments
83008db 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: nodejs#20018
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem. labels Apr 14, 2018
@ryzokuken
Copy link
Contributor

cc @mcollina @nodejs/http2

vsemozhetbyt
vsemozhetbyt reviewed Apr 14, 2018
View reviewed changes
doc/api/http2.md Outdated
@@ -128,7 +128,8 @@ solely on the API of the `Http2Session`.
added: v8.4.0
-->

The `'close'` event is emitted once the `Http2Session` has been destroyed.
The `'close'` event is emitted once the `Http2Session` has been destroyed. It
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!
Feel free to ignore, but maybe it would be a bit more clear if we write "Its listener does not expect any arguments"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Good idea. I edited the wording to reflect the change.

@indranil
doc: changed wording to better state listener
158aeb4 
Added wording to better state that it is in fact the listener
that is not expecting any arguments

Fixes: nodejs#20018
@vsemozhetbyt vsemozhetbyt added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 14, 2018
@vsemozhetbyt
Copy link
Contributor

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite/519/

@vsemozhetbyt vsemozhetbyt added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 14, 2018
@ryzokuken
Copy link
Contributor

ryzokuken commented Apr 14, 2018 •
edited
Loading

Because this was fast-tracked it can be landed, right? (With perhaps a couple more reviews)

trivikr pushed a commit that referenced this pull request Apr 14, 2018
@indranil @trivikr
doc: close event does not take arguments
594e59d 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: #20018

PR-URL: #20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@trivikr
Copy link
Member

trivikr commented Apr 14, 2018 •
edited
Loading

Landed in 594e59d

Congrats @indranil on your first contribution to Node.js core! 🎉🎉🎉

@trivikr trivikr closed this Apr 14, 2018
@indranil indranil deleted the edit-http2-docs-specify-args-close-event branch April 15, 2018 02:05
@indranil indranil restored the edit-http2-docs-specify-args-close-event branch April 15, 2018 02:07
jasnell pushed a commit that referenced this pull request Apr 16, 2018
@indranil @jasnell
doc: close event does not take arguments
98008dc 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: #20018

PR-URL: #20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
@indranil @BridgeAR
doc: close event does not take arguments
80e496f 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: nodejs#20018

PR-URL: nodejs#20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Aug 23, 2018
@indranil @kjin
doc: close event does not take arguments
d2691d6 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: nodejs#20018

PR-URL: nodejs#20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Sep 11, 2018
@indranil @kjin
doc: close event does not take arguments
f5a9fec 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: nodejs#20018

PR-URL: nodejs#20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Sep 19, 2018
@indranil @kjin
doc: close event does not take arguments
713d357 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: nodejs#20018

PR-URL: nodejs#20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Oct 16, 2018
@indranil @kjin
doc: close event does not take arguments
f3f4ee6 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: nodejs#20018

PR-URL: nodejs#20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Oct 17, 2018
@indranil @BethGriggs
doc: close event does not take arguments
c0d1423 
Explicitly added in the docs that the close event does not expect
any arguments when invoked.

Fixes: #20018

Backport-PR-URL: #22850
PR-URL: #20031
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Oct 30, 2018
Merged
@beevelop beevelop mentioned this pull request Nov 20, 2018
Closed
This was referenced Jun 3, 2020
Open
Open
@snyk-bot snyk-bot mentioned this pull request Jun 14, 2020
Open
@kissofire kissofire mentioned this pull request Nov 14, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr approved these changes

@ryzokuken ryzokuken ryzokuken approved these changes

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

State explicitly in docs that 'close' does not take any arguments
6 participants
@indranil @ryzokuken @vsemozhetbyt @trivikr @jasnell @nodejs-github-bot