-
Notifications
You must be signed in to change notification settings - Fork 1
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
First minimal version adding response examples to Postman collections. #76
Conversation
* Need to add test * Need to handle codes properly * Need to ensure responses are placed in the right location
* Not working with GET yet * Commenting out relevant test for now as tests are malleable
* Refactors mustache template to avoid duplication
Awesome work, I left some comments to consider/address |
@@ -0,0 +1,23 @@ | |||
package com.tweesky.cloudtools.codegen; |
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.
@gcatanese I moved to something more generic, which should be better.
However, it does modify the behaviour somehow (for example "success" default cases).
can you confirm whether that is desired?
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, an unexpected status code thrown an exception, which is probably better.
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.
ah, forgot that case, I'll an extra test!
Hey @gcatanese before merging this do I need to do anything more? Create the new docker layer or something? |
Upon merge on |
Need to add testNeed to handle codes properlyNeed to ensure responses are placed in the right location