doc: improve callback documentation of http2Stream.pushstream() #18258
Conversation
Improve documentation of callback signature of http2Stream.pushStream() function to align with the changes made in nodejs#17406. Fixes: nodejs#18198 Refs: nodejs#17406
|
Hi, @nephross. Welcome to the Node.js project and thank you for your contribution! It seems your local git email settings is out of sync with the GitHub email settings, so GitHub does not associate your commit with your account. To fix this, see "Setting up your local environment" (the last paragraph in particular). |
|
@vsemozhetbyt Thank you for pointing it out, found the error and fixed for my local repo, but how do I fix it for the commit already pushed? |
|
Hmm. I thought this would be updated automatically. Maybe you can try to click on this "?" sign here and re-enter the email?
|
doc/api/http2.md
Outdated
| @@ -1197,8 +1197,10 @@ added: v8.4.0 | |||
| **Default:** `false` | |||
| * `parent` {number} Specifies the numeric identifier of a stream the newly | |||
| created stream is dependent on. | |||
| * `callback` {Function} Callback that is called once the push stream has been | |||
| initiated. | |||
| * `callback` {Function} Callback that is called once the push stream has been initiated. | |||
There was a problem hiding this comment.
Nit: please wrap lines at 80 chars.
|
@vsemozhetbyt I don't see the ? unfortunately. I have tried amending the commit author but it is not reflecting upstream. |
doc/api/http2.md
Outdated
| * `callback` {Function} Callback that is called once the push stream has been | ||
| initiated. | ||
| * `callback` {Function} Callback that is called once the push stream has been | ||
| initiated. |
There was a problem hiding this comment.
A nit: I wonder if the hard line break is rendered here due to missing indentation.
doc/api/http2.md
Outdated
| initiated. | ||
| * `err` {Error} | ||
| * `pushStream` {[`ServerHttp2Stream`][]} The returned pushStream object. | ||
| * `headers` {[Headers Object][]} Headers object the pushStream was initiated with. |
There was a problem hiding this comment.
A nit: this line also needs wrapping (currently 84 characters).
Sorry for the pettiness) This can be fixed by a commit lander if it is not convenient for you to amend the commit or push more commits)
There was a problem hiding this comment.
it's all good. My first pr, so I'm here to learn.
|
@nephross That the second commit has the proper info now, I think this suffices for proper PR attribution. Thank you! |
|
Memo for the lander: I am not sure how will squashing go with the different commit authors and how this will affect the promotion to Contributors, so we may need to be a bit more careful here. |
|
Landed in b886c42 Thank you! |
Improve documentation of callback signature of http2Stream.pushStream() function to align with the changes made in nodejs#17406. PR-URL: nodejs#18258 Fixes: nodejs#18198 Refs: nodejs#17406 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Improve documentation of callback signature of http2Stream.pushStream() function to align with the changes made in nodejs#17406. PR-URL: nodejs#18258 Fixes: nodejs#18198 Refs: nodejs#17406 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Improve documentation of callback signature of
http2Stream.pushStream() function to align with
the changes made in #17406.
Fixes: #18198
Refs: #17406
Checklist
Affected core subsystem(s)
doc