Skip to content

doc: add parameters for Http2Session:stream event - #20547

Closed
ryzokuken wants to merge 2 commits into
nodejs:masterfrom
ryzokuken:docs/http2-6
Closed

doc: add parameters for Http2Session:stream event#20547
ryzokuken wants to merge 2 commits into
nodejs:masterfrom
ryzokuken:docs/http2-6

Conversation

@ryzokuken

Copy link
Copy Markdown
Contributor

Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.

Refs: nodejs/help#877 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

P.S. I was kinda confused regarding the type of the flags variable, so I set it to Number for now.

/cc @nodejs/http2 @mcollina@vsemozhetbyt

Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
@ryzokukenryzokuken self-assigned this May 5, 2018
@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem. labels May 5, 2018
@ryzokuken

Copy link
Copy Markdown
ContributorAuthor

Also, please respond with 👍 to approve fast-tracking.

@ryzokuken

Copy link
Copy Markdown
ContributorAuthor

Comment threaddoc/api/http2.md Outdated
* `stream` {Http2Stream} A reference to the stream
* `headers` {HTTP/2 Headers Object} An object describing the headers
* `flags` {number} The associated numeric flags
* `rawHeaders` {Array} An array containing the raw header name value pairs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this actually supposed to be public? @nodejs/http2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure what is the convention here, but maybe name value -> name-value or name/value or name:value (with or without spaces, I am not sure as well)?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes it's correct. rawHeaders is exposed as request.rawHeaders.

@ryzokukenryzokukenMay 6, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@vsemozhetbyt I think it could be either depending on the key:value delimiter? Let me check what the delimiter is in this particular case.

@addaleaxaddaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM but have a question about rawHeaders

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@vsemozhetbytvsemozhetbyt added fast-track PRs that do not need to wait for 48 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 5, 2018
@ryzokuken

Copy link
Copy Markdown
ContributorAuthor

P.S. Checked it out by running an actual example, and the strings weren't name:value pairs per se, but every name and value is a separate value in the array. Making appropriate changes to the docs and landing this.

@ryzokuken

Copy link
Copy Markdown
ContributorAuthor

@vsemozhetbyt take another look?

@vsemozhetbyt

Copy link
Copy Markdown
Contributor

@ryzokuken

Copy link
Copy Markdown
ContributorAuthor

CI passed, landing this.

@ryzokuken

Copy link
Copy Markdown
ContributorAuthor

BTW, @vsemozhetbyt I wanted to talk to you about a thing or two regarding docs, but couldn't find you on IRC. How can I reach out to you?

@ryzokuken

Copy link
Copy Markdown
ContributorAuthor

Landed in 29ccf6e

Trott pushed a commit to Trott/io.js that referenced this pull request May 7, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: nodejs#20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@vsemozhetbyt

Copy link
Copy Markdown
Contributor

@ryzokuken I do not monitor IRC, but the email in the GitHub profile is live)

MylesBorins pushed a commit that referenced this pull request May 8, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: #20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request May 8, 2018
MylesBorins pushed a commit that referenced this pull request May 8, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: #20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 9, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: #20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Aug 23, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: nodejs#20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Sep 11, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: nodejs#20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Sep 19, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: nodejs#20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request Oct 16, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
PR-URL: nodejs#20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
BethGriggs pushed a commit that referenced this pull request Oct 17, 2018
Add parameters for the callback for the Http2Session:stream event
inline with the pattern in the rest of the documentation.
Refs: nodejs/help#877 (comment)
Backport-PR-URL: #22850
PR-URL: #20547
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Oct 30, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.docIssues and PRs related to the documentations.fast-trackPRs that do not need to wait for 48 hours to land.http2Issues or PRs related to the http2 subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@ryzokuken@vsemozhetbyt@mcollina@addaleax@trivikr@nodejs-github-bot