-
Notifications
You must be signed in to change notification settings - Fork 91
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
feat: add warning, non-204 success response should have response body #127
Conversation
e0a2d01
to
1f22289
Compare
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
==========================================
+ Coverage 92.65% 92.66% +0.01%
==========================================
Files 56 56
Lines 2055 2060 +5
==========================================
+ Hits 1904 1909 +5
Misses 151 151
Continue to review full report at Codecov.
|
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.
The code looks good! I just have a few comments to address before merging
1f22289
to
95c6684
Compare
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.
This looks great. I forgot one thing in my last review - I don't believe this rule is explicitly required by the OpenAPI Spec. Since that's the case, the rule should be documented and configurable. You will need to:
- Update the rules section of the readme
- Update the rule defaults section of the readme
- Update the defaults file
- Check for configuration when it's time to check for the rule
95c6684
to
fa08d63
Compare
fa08d63
to
bea76f2
Compare
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.
Last change!
bea76f2
to
76eb308
Compare
- added logic to check for response bodies in non-204 success responses in the oas3 plugin - added a test to check that a non-204 success response without response body generates a warning - added a test to check that multiple non-204 success responses without response bodies generates multiple warnings - updated the README.md to help users resolve potential issue with `npm run link` - fixed API definitions and tests that define a non-204 success without adding a response body - ran `npm run fix` to fix styling issues - updated variable names to be more clear - added to tests to compare expected messages and paths - added no_response_body rule and set the default value to 'warning'
76eb308
to
3a4e785
Compare
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.18.0](v0.17.1...v0.18.0) (2020-02-07) ### Features * add warning, non-204 success response should have response body ([#127](#127)) ([90689c8](90689c8))
🎉 This PR is included in version 0.18.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Related Issue: arf/planning-sdk-squad/issues/560
Changes:
npm run link