doc: Make createPushResponse() more detailled. · Pull Request #22366 · nodejs/node · GitHub

Ref: #22322

In summary:
We don't know what will return when successful or failure for
the callback of the function. So make it more detailled.

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

@nodejs/documentation @nodejs/http2

Grammar/spelling nit:

For the closed stream, the callback

Any spelling wrong or……?

I meant omit the And, and lower-case the.

OK. Thanks. I'll also collect other suggestions and do modifications together :)

LGTM

hmm... the when the stream is closed bit is a bit confusing.

The callback is invoked either (a) after the pushed Http2Stream instead has been created and is ready for use or (b) after the attempt to create the pushed Http2Stream has failed or has been rejected.

Well……according to what you suggested, I've included these points in my doc modifications :)

Needs additional edits. See comment ^^

Http2Stream -> `Http2Stream`
We usually align parameter description lines, so this and the next line need 2 space indent.

OK

For 1st: I've only meant to add backticks)

ok, I've found it. Thanks!

Consistency nit: it seems we do not add periods if the description is not a full sentence (see headers description above).

OK

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

The last bit, , or the state of Http2ServerRequest is closed should be clarified. , or the state of Http2ServerRequest is closed prior to calling the pushStream() method. would be clearer.

OK, it's been fixed.

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

@Maledong can you rebase against master? There is the need of a commit there to make CI pass.

@mcollina：OK, I'll rebase and have a submit :)

Ref: #22322 In summary: We don't know what will return when successful or failure for the callback of the function. So make it more detailled.

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

Landed in e7b4ba9

We don't know what will return when successful or failure for the callback of the function. So this commit makes it more detailled. PR-URL: #22366 Refs: #22322 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Thanks all!

We don't know what will return when successful or failure for the callback of the function. So this commit makes it more detailled. PR-URL: #22366 Refs: #22322 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

We don't know what will return when successful or failure for the callback of the function. So this commit makes it more detailled. PR-URL: nodejs#22366 Refs: nodejs#22322 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

We don't know what will return when successful or failure for the callback of the function. So this commit makes it more detailled. Backport-PR-URL: #22850 PR-URL: #22366 Refs: #22322 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

nodejs-github-bot added doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem. labels Aug 17, 2018

sebdeckers reviewed Aug 17, 2018

View reviewed changes

mcollina approved these changes Aug 17, 2018

View reviewed changes

jasnell reviewed Aug 17, 2018

View reviewed changes

jasnell requested changes Aug 17, 2018

View reviewed changes

vsemozhetbyt reviewed Aug 18, 2018

View reviewed changes

jasnell reviewed Aug 22, 2018

View reviewed changes

trivikr approved these changes Aug 22, 2018

View reviewed changes

jasnell approved these changes Aug 22, 2018

View reviewed changes

doc: Make createPushResponse() more detailled.

2dc6dce

Ref: #22322 In summary: We don't know what will return when successful or failure for the callback of the function. So make it more detailled.

vsemozhetbyt closed this Aug 23, 2018

vsemozhetbyt reopened this Aug 23, 2018

mcollina closed this Aug 23, 2018

ghost deleted the FixDoc branch August 23, 2018 09:13

targos mentioned this pull request Sep 5, 2018

Release proposal: v10.10.0 #22716

Merged

This was referenced Sep 6, 2018

2018-09-06 Version 10.10.0 (Current) @targos beevelop/docker-android-nodejs#236

Closed

2018-09-06 Version 10.10.0 (Current) @targos beevelop/docker-nodejs#221

Closed

BethGriggs mentioned this pull request Oct 30, 2018

v8.13.0 proposal #23974

Merged

beevelop mentioned this pull request Nov 20, 2018

2018-11-20 Version 8.13.0 'Carbon' (LTS) @MylesBorins prepared by @BethGriggs beevelop/docker-android-nodejs#250

Closed

snyk-bot mentioned this pull request Aug 15, 2021

[Snyk] Fix for 1 vulnerabilities ron-a20/node#39

Open

molllyn1 mentioned this pull request Mar 22, 2024

[Snyk] Fix for 2 vulnerabilities molllyn1/node#106

Open

137717unity mentioned this pull request Mar 22, 2024

[Snyk] Fix for 2 vulnerabilities 137717unity/node#122

Open

kissofire mentioned this pull request Mar 22, 2024

[Snyk] Fix for 2 vulnerabilities kissofire/node#124

Open

aliceUnhinged613 mentioned this pull request Mar 22, 2024

[Snyk] Fix for 2 vulnerabilities aliceUnhinged613/node#123

Open

ron-a20 mentioned this pull request Mar 22, 2024

[Snyk] Fix for 2 vulnerabilities ron-a20/node#121

Open

wedataintelligence mentioned this pull request Mar 23, 2024

[Snyk] Fix for 2 vulnerabilities wedataintelligence/node#105

Open

doc: Make createPushResponse() more detailled. · Pull Request #22366 · nodejs/node · GitHub

Uh oh!

Uh oh!

doc: Make createPushResponse() more detailled. - #22366

doc: Make createPushResponse() more detailled.#22366ghost wants to merge 1 commit intomasternodejs/node:masterfrom unknown repositoryCopy head branch name to clipboard

Conversation

ghost commented Aug 17, 2018

Uh oh!

Trott commented Aug 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Aug 18, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt commented Aug 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcollina commented Aug 22, 2018

Uh oh!

mcollina commented Aug 22, 2018

Uh oh!

ghost commented Aug 23, 2018

Uh oh!

vsemozhetbyt commented Aug 23, 2018

Uh oh!

mcollina commented Aug 23, 2018

Uh oh!

ghost commented Aug 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

doc: Make createPushResponse() more detailled.#22366
ghost wants to merge 1 commit into
masterfrom
unknown repository

ghost Aug 18, 2018 •
edited by ghost
Loading