-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: Improve doc for Http2 headers object #21296
Conversation
Add more details regarding processing and data type of incoming headers in Http2.
@nodejs/documentation |
I'm not convinced most of this belongs in the http2 documentation since we're just following the spec (and it applies to http too). I am potentially in favour of documenting the type of the :status header. |
@apapirovski My start point in this was the
I'm fine with removing some of the details, replace it by references to http or http2 spec (or even code). |
doc/api/http2.md
Outdated
discarded. | ||
* `set-cookie` is a string if present once or an array in case duplicates | ||
are present. | ||
* `cookie` the values are joined together with '; '. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a colon after `cookie`
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
I was thinking once more regarding the difference in handling of Till now this diff is not documented and http2 is experimental so this gap could be closed easily. @nodejs/http2 any opinion in this? |
@apapirovski What do think about following: |
@Flarna I would be fine with that but this has approvals as is so don't feel pressured to make the change. I think I'm in the minority in my opinion :) |
@apapirovski I took a fast look into the history and it seems the headerlist is unchanged since ~9 month. Therefore I think it's ok to mention all headers in doc as the risk of getting out of sync is low. |
Let's land this, say, tomorrow if there are no strong objections? |
Landed in 8132413 |
Add more details regarding processing and data type of incoming headers in Http2. PR-URL: #21296 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Add more details regarding processing and data type of incoming headers in Http2. PR-URL: #21296 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Add more details regarding processing and data type of incoming headers in Http2. PR-URL: nodejs#21296 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Add more details regarding processing and data type of incoming headers in Http2. PR-URL: nodejs#21296 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Add more details regarding processing and data type of incoming headers in Http2. PR-URL: nodejs#21296 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Add more details regarding processing and data type of incoming headers in Http2.
Checklist