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.

Sign up for GitHub

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

Jump to bottom

doc: reword message.headers #3814

Closed
wants to merge 1 commit into from
Closed

doc: reword message.headers #3814

wants to merge 1 commit into from

Conversation

tflanagan
Copy link
Contributor

message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Issue: #3146

'use strict';

const http = require('http');

http.createServer(function(req, res){
    req.headers.host = 'anotherhost';

    res.writeHead(200, {'Content-Type': 'text/plain'});
    res.end(req.headers.host);
}).listen(8124);

console.log('Server running at http://127.0.0.1:8124/');

// "anotherhost"

@tflanagan
doc: reword message.headers
7d2b47b 
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Issue: nodejs#3146
@mscdex mscdex added http Issues or PRs related to the http subsystem. doc Issues and PRs related to the documentations. labels Nov 13, 2015
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

LGTM.
This brings up a good point tho about whether these are supposed to be read only but aren't (a code bug) or whether it's just that the doc doesn't match the implementation. @indutny @mikeal

@indutny
Copy link
Member

indutny commented Nov 13, 2015

@jasnell they are kind of read-only (ideally), but in fact this is expensive to enforce. So the key-value pairs seems to be precise definition.

@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Ok, then it's a doc bug. 👍

@r-52
Copy link
Contributor

r-52 commented Nov 13, 2015

LGTM

jasnell pushed a commit that referenced this pull request Nov 13, 2015
@tflanagan @jasnell
doc: reword message.headers to indicate they are not read-only
934149a 
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Landed in 934149a

@jasnell jasnell closed this Nov 13, 2015
@tflanagan tflanagan deleted the doc-http-headers-kvp branch November 13, 2015 20:16
Fishrock123 pushed a commit that referenced this pull request Nov 17, 2015
@tflanagan @Fishrock123
doc: reword message.headers to indicate they are not read-only
5d1faa2 
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 17, 2015
@tflanagan
doc: reword message.headers to indicate they are not read-only
fc629c2 
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

landed in v4.x-staging in fc629c2

@MylesBorins MylesBorins mentioned this pull request Nov 17, 2015
Closed
rvagg pushed a commit that referenced this pull request Dec 4, 2015
@tflanagan @rvagg
doc: reword message.headers to indicate they are not read-only
1d5cbe7 
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell mentioned this pull request Dec 17, 2015
Closed
jasnell pushed a commit that referenced this pull request Dec 17, 2015
@tflanagan @jasnell
doc: reword message.headers to indicate they are not read-only
a36a5b6 
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
@tflanagan @jasnell
doc: reword message.headers to indicate they are not read-only
630ddfd 
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31802/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tflanagan @jasnell @indutny @r-52 @MylesBorins @nodejs-github-bot @mscdex