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

Fix http status codes #20700

Closed
wants to merge 2 commits into from
Closed

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented May 13, 2018

Fixes #20286.

I have used the @mvasilkov 's patch and @mithunsasidharan 's advice to make Node.js compliant with the latest HTCPCP-TEA (RFC 7168, an extension to HTCPCP to allow for pots to provide networked tea-brewing facilities).

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Update the message to be consistent with RFC 7168, and the rest of the statuses.
@nodejs-github-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label May 13, 2018
Copy link
Member

@apapirovski apapirovski left a comment

Choose a reason for hiding this comment

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

LGTM

@apapirovski
Copy link
Member

Copy link
Member

@apapirovski apapirovski left a comment

Choose a reason for hiding this comment

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

Actually, sorry, looks like Multiple Choices is still correct as per https://tools.ietf.org/html/rfc7231#section-6.4.1

Perhaps change it back and adjust the comment to point to RFC7231 so this doesn't come up in the future?

@aduh95
Copy link
Contributor Author

aduh95 commented May 14, 2018

The JS linter doesn’t pass, I need to rephrase my comment. I’m taking care of that later today!

EDIT: Done.

@BridgeAR
Copy link
Member

@BridgeAR
Copy link
Member

What is this semver wise? 😄

@aduh95
Copy link
Contributor Author

aduh95 commented May 18, 2018

What is this semver wise? 😄

That would be major right? I mean, we certainly wouldn't risk to break all the coffee pot controlling installations out there, would we? Those people deserve their coffee, even if they use an old version of the spec! 😄

apapirovski pushed a commit that referenced this pull request May 22, 2018
Update the message to be consistent with RFC 7168. Add a note to
"Multiple Choices" regarding RFC 7231 superseding RFC 7168.

PR-URL: #20700
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>t

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@apapirovski
Copy link
Member

Landed in 464852b

@mvasilkov and @aduh95 you should both be Contributors now! Thanks for the contribution.

MylesBorins pushed a commit that referenced this pull request May 22, 2018
Update the message to be consistent with RFC 7168. Add a note to
"Multiple Choices" regarding RFC 7231 superseding RFC 7168.

PR-URL: #20700
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>t

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@addaleax addaleax mentioned this pull request May 22, 2018
MylesBorins pushed a commit that referenced this pull request May 23, 2018
Update the message to be consistent with RFC 7168. Add a note to
"Multiple Choices" regarding RFC 7231 superseding RFC 7168.

PR-URL: #20700
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>t

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95 aduh95 deleted the fix_http_status_codes branch May 24, 2018 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the capitalization of HTTP 418 I'm a Teapot
9 participants