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.

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

Documentation for Http2ServerRequest properties #23825

Closed
mgjm opened this issue Oct 22, 2018 · 5 comments
Closed

Documentation for Http2ServerRequest properties #23825

mgjm opened this issue Oct 22, 2018 · 5 comments
Labels
doc Issues and PRs related to the documentations. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. http2 Issues or PRs related to the http2 subsystem.

Comments

@mgjm
Copy link
Contributor

mgjm commented Oct 22, 2018

Is your feature request related to a problem? Please describe.
The http2.Http2ServerRequest has some useful properties that are undocumented. But IMHO, they should be part of the public API.

Describe the solution you'd like
The following properties could become part of the http2 module documentation:
method, authority, scheme, url
(See lib/internal/http2/compat.js)

Describe alternatives you've considered
The alternative is to use the headers object:

// alternative to: req.authority
req.headers[':authority']

PR?
If these properties should be documented and therefore part of the public API, I am happy to create the corresponding documentation in a PR.

@Trott Trott added doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem. labels Nov 4, 2018
@Trott
Copy link
Member

Trott commented Nov 5, 2018

@nodejs/documentation @nodejs/http2

@Trott
Copy link
Member

Trott commented Nov 13, 2018

Looks like method and url are documented. Still needs authority and scheme. I don't see nay reason these shouldn't be documented.

@Trott Trott added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Nov 13, 2018
@kenigbolo
Copy link
Contributor

@Trott Point me in the right direct and I'd take this on asap.

@mgjm
Copy link
Contributor Author

mgjm commented Nov 13, 2018

@kenigbolo I think the documentation of Http2ServerRequest.method is a good example. And the source of the documentation is at doc/api/http2.md.

And the contributing guide for PRs is a good starting point. But for a "documentation only" PR there is no need to compile and test node itself. It should be enough to test the documentation with make test-doc (instead of a full ./configure && make -j4 test).

And if make test-doc fails with "Could not find executable. Should be out/Release/node" you can link to your global node as a temporary fix:
mkdir out/Release && ln -s `which node` out/Release/node

@kenigbolo
Copy link
Contributor

kenigbolo commented Dec 1, 2018

@mgjm Thanks a lot. I'd get on it today. Can this be assigned to me?

kenigbolo added a commit to kenigbolo/node that referenced this issue Dec 2, 2018
Add documentation for  http2 request `authority` and `scheme` psuedo headers of http2 request
kenigbolo added a commit to kenigbolo/node that referenced this issue Dec 2, 2018
This pull request covers adding documentation for authority and scheme

Fixes: nodejs#23825
kenigbolo added a commit to kenigbolo/node that referenced this issue Dec 2, 2018
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

Fixes: nodejs#23825
BridgeAR pushed a commit that referenced this issue Dec 6, 2018
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

PR-URL: #24777
Fixes: #23825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BridgeAR pushed a commit that referenced this issue Dec 7, 2018
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

PR-URL: #24777
Fixes: #23825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BridgeAR pushed a commit that referenced this issue Dec 7, 2018
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

PR-URL: #24777
Fixes: #23825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

PR-URL: nodejs#24777
Fixes: nodejs#23825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

PR-URL: #24777
Fixes: #23825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 20, 2019
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

PR-URL: #24777
Fixes: #23825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
This pull request adds the request psuedo headers authority
and scheme to the http2 documentation

PR-URL: #24777
Fixes: #23825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants