-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add validation for use of 101 status code #237
Conversation
@@ -77,13 +77,20 @@ module.exports.validate = function({ resolvedSpec }, config) { | |||
} | |||
} | |||
// validate all success codes | |||
if (!successCodes.length) { | |||
if (!successCodes.length && !("101" in obj)) { |
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.
I think I would have implemented this differently. I would have changed getResponseCodes
to return 101
in successCodes
, since we want to treat it like one. I'm curious why you chose this implementation instead.
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.
I actually did that first but it would have required changes to the check below that verifies that 204 is the only success code sans body.
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.
I'm good with this then. We shouldn't over-think this.
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.
Looks good! 👍
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.
Looks good! 👍
## [0.34.2](v0.34.1...v0.34.2) (2021-02-18) ### Bug Fixes * add validation for use of 101 status code ([#237](#237)) ([52e1319](52e1319)) * run travis deploy step on main branch ([#239](#239)) ([6b5abbb](6b5abbb)) * update releaserc to use main branch ([#238](#238)) ([4b1569b](4b1569b)) * update semantic-release dependencies ([#240](#240)) ([6edf60c](6edf60c))
🎉 This PR is included in version 0.34.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Currently
101
is not considered a success and therefore it's not valid for an operation to support101
without also supporting a2xx
code. In fact, we'd prefer to validate that101
MUST NOT be used along with a2xx
response code.