FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Makes http(2) response.writehead return this by qubyte · Pull Request #25974 · nodejs/node · GitHub

/ node Public

Makes http(2) response.writehead return this - #25974

Closed
qubyte wants to merge 7 commits into
nodejs:masterfrom
qubyte:http-response-writehead-return-this
ConversationCommits (7)ChecksFiles changed
Closed

Makes http(2) response.writehead return this#25974
qubyte wants to merge 7 commits into
nodejs:masterfrom
qubyte:http-response-writehead-return-this

Conversation

qubyte commented Feb 7, 2019

Copy link
Copy Markdown
Contributor

Addresses #25935.

I chose in the end to go with the lightest touch and only update writeHead (this is the most useful to change in my experience). I'm happy to extend this to res.setHeader too, but it may suggest updating other similar methods (flushHeaders, removeHeader, etc.)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

nodejs-github-bot added dont-land-on-v6.x http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. labels Feb 7, 2019
qubyte commented Feb 7, 2019
View reviewed changes
Comment thread lib/internal/http2/compat.js Outdated

qubyte Feb 7, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I couldn't find the code which this refers to. The HTTP version of writeHead appears to have no explicit returns.

addaleax approved these changes Feb 7, 2019
View reviewed changes
Comment thread doc/api/http.md Outdated
Comment thread doc/api/http2.md Outdated
addaleax and others added 2 commits February 7, 2019 00:55
http: update to documentation
1ce1dcc 
Addresses PR feedback.

Co-Authored-By: qubyte <mark.s.everitt@gmail.com>
http2: update to documentation
3a4d057 
Addresses PR feedback.

Co-Authored-By: qubyte <mark.s.everitt@gmail.com>
vsemozhetbyt reviewed Feb 7, 2019
View reviewed changes

vsemozhetbyt left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you. Some doc nits.

Comment thread doc/api/http2.md Outdated
Comment thread doc/api/http.md Outdated
Comment thread doc/api/http2.md Outdated
vsemozhetbyt and others added 2 commits February 7, 2019 01:39
http2: update to documentation
2f476b6 
Addresses PR feedback.

Co-Authored-By: qubyte <mark.s.everitt@gmail.com>
http: update to documentation
c5d4090 
Addresses PR feedback.

Co-Authored-By: qubyte <mark.s.everitt@gmail.com>
http2: update to documentation
df45bdc 
Addresses PR feedback.

Co-Authored-By: qubyte <mark.s.everitt@gmail.com>

lpinca commented Feb 9, 2019

Copy link
Copy Markdown
Member

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

addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 9, 2019

addaleax commented Feb 9, 2019

Copy link
Copy Markdown
Member

Landed in f93df51, 1aa11e1 – thanks for the PR! 🎉

addaleax closed this Feb 9, 2019
addaleax pushed a commit that referenced this pull request Feb 9, 2019
http: makes response.writeHead return the response
f93df51 
Fixes: #25935

PR-URL: #25974
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax pushed a commit that referenced this pull request Feb 9, 2019
http2: makes response.writeHead return the response
1aa11e1 
Fixes: #25935

PR-URL: #25974
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

qubyte commented Feb 9, 2019

Copy link
Copy Markdown
Contributor Author

Thanks for accepting! 😁

targos pushed a commit that referenced this pull request Feb 10, 2019
http: makes response.writeHead return the response
a5247cc 
Fixes: #25935

PR-URL: #25974
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Feb 10, 2019
http2: makes response.writeHead return the response
237b5e6 
Fixes: #25935

PR-URL: #25974
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos mentioned this pull request Feb 14, 2019
Merged
This was referenced Feb 15, 2019
Closed
Closed
Closed
Closed
Closed
Closed
qubyte deleted the http-response-writehead-return-this branch February 27, 2019 22:54
BethGriggs pushed a commit that referenced this pull request Oct 7, 2019
http: makes response.writeHead return the response
ccf2823 
Fixes: #25935

PR-URL: #25974
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs pushed a commit that referenced this pull request Oct 7, 2019
http2: makes response.writeHead return the response
9258496 
Fixes: #25935

PR-URL: #25974
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs mentioned this pull request Oct 7, 2019
Merged
BethGriggs added a commit that referenced this pull request Oct 22, 2019
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
5dae3ef 
Notable changes:

* crypto:
  * add support for chacha20-poly1305 for AEAD (chux0519)
    #24081
  * increase maxmem range from 32 to 53 bits (Tobias Nießen)
    #28799
* deps:
  * update npm to 6.11.3 (claudiahdz)
    #29430
  * upgrade openssl sources to 1.1.1d (Sam Roberts)
    #29921
* dns:
  * remove dns.promises experimental warning (cjihrig)
    #26592
* fs:
  * remove experimental warning for fs.promises (Anna Henningsen)
    #26581
* http:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* http2:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* n-api:
  * make func argument of napi\_create\_threadsafe\_function optional
    (legendecas)
    #27791
  * mark version 5 N-APIs as stable (Gabriel Schulhof)
    #29401
  * implement date object (Jarrod Connolly)
    #25917
* process:
  * add --unhandled-rejections flag (Ruben Bridgewater)
    #26599
* stream:
  * implement Readable.from async iterator utility (Guy Bedford)
    #27660
  * make Symbol.asyncIterator support stable (Matteo Collina)
    #26989

PR-URL: #29875
BethGriggs added a commit that referenced this pull request Oct 22, 2019
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
afbbcb0 
Notable changes:

* crypto:
  * add support for chacha20-poly1305 for AEAD (chux0519)
    #24081
  * increase maxmem range from 32 to 53 bits (Tobias Nießen)
    #28799
* deps:
  * update npm to 6.11.3 (claudiahdz)
    #29430
  * upgrade openssl sources to 1.1.1d (Sam Roberts)
    #29921
* dns:
  * remove dns.promises experimental warning (cjihrig)
    #26592
* fs:
  * remove experimental warning for fs.promises (Anna Henningsen)
    #26581
* http:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* http2:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* n-api:
  * make func argument of napi\_create\_threadsafe\_function optional
    (legendecas)
    #27791
  * mark version 5 N-APIs as stable (Gabriel Schulhof)
    #29401
  * implement date object (Jarrod Connolly)
    #25917
* process:
  * add --unhandled-rejections flag (Ruben Bridgewater)
    #26599
* stream:
  * implement Readable.from async iterator utility (Guy Bedford)
    #27660
  * make Symbol.asyncIterator support stable (Matteo Collina)
    #26989

PR-URL: #29875
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes
lpinca lpinca approved these changes
+1 more reviewer
vsemozhetbyt vsemozhetbyt left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL