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

doc: message.header duplication correction #3997

Closed
wants to merge 1 commit into from

Conversation

bengl
Copy link
Member

@bengl bengl commented Nov 24, 2015

The old version of the text here was slightly incorrect (probably how it used to work, so it's now it's slightly more vague, deferring the details to the section in which they're already fully described.

@mscdex mscdex added http Issues or PRs related to the http subsystem. doc Issues and PRs related to the documentations. labels Nov 24, 2015
single value is represented on the parsed object.
When headers are duplicated, either they're concatenated with `', '`,
only the first one is taken, or they're presented as an array, depending
on the header. See [message.headers][].
Copy link
Member

Choose a reason for hiding this comment

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

LGTM but maybe link directly to message.headers without trying to describe the logic first? Something like See [message.headers][] for details on how duplicate headers are handled. That way it can't go out of sync.

@bnoordhuis
Copy link
Member

LGTM with a suggestion. If other collaborators feel it's good as-is, go ahead and land it.

@evanlucas
Copy link
Contributor

I think linking will be the better solution. LGTM other than that

@cjihrig
Copy link
Contributor

cjihrig commented Nov 24, 2015

LGTM with the same comment.

The old version of the text here was slightly incorrect, so it
just defers the details to the section in which they're fully
described.
@bengl
Copy link
Member Author

bengl commented Nov 24, 2015

Cool, shortened it down to just what @bnoordhuis suggested.

@bnoordhuis
Copy link
Member

Thanks Bryan, landed in 6f87aa9.

@bnoordhuis bnoordhuis closed this Nov 24, 2015
bnoordhuis pushed a commit that referenced this pull request Nov 24, 2015
The old version of the text here was slightly incorrect, so it
just defers the details to the section in which they're fully
described.

PR-URL: #3997
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this pull request Dec 1, 2015
The old version of the text here was slightly incorrect, so it
just defers the details to the section in which they're fully
described.

PR-URL: #3997
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
rvagg pushed a commit that referenced this pull request Dec 4, 2015
The old version of the text here was slightly incorrect, so it
just defers the details to the section in which they're fully
described.

PR-URL: #3997
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
rvagg pushed a commit that referenced this pull request Dec 5, 2015
The old version of the text here was slightly incorrect, so it
just defers the details to the section in which they're fully
described.

PR-URL: #3997
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
The old version of the text here was slightly incorrect, so it
just defers the details to the section in which they're fully
described.

PR-URL: #3997
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
The old version of the text here was slightly incorrect, so it
just defers the details to the section in which they're fully
described.

PR-URL: #3997
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.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. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants