-
Notifications
You must be signed in to change notification settings - Fork 61
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(Catalog Management): Re-gen SDK #205
Conversation
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.
Did you really intend to delete the integration tests and examples?
I didn't intend to delete the examples. The other integration tests files are old though, so I thought we could delete those. |
I'm ok with you deleting the old int test files as long as the new file contains a set of integration tests that exercise each operation of the service. Re: the working examples, those need to be put back in place as well. I skimmed through the new integration test file and there are a LOT of test values being used for properties that are not "real", such as "testString", etc. These mock values are being used by the generator because the API definition lacks sufficient "example" values for request bodies, responses, schemas, and parameters. |
912408f
to
f66835a
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.
Changes look good.
Please post a screenshot of a clean run of the int tests and examples.
i.e. cd catalogmanagementv1
then go test -tags=integration,examples
.
Once we have that I can merge.
Edit: Also, the branch needs to be updated with the latest from "main" before it can be merged in.
a461b3d
to
6018093
Compare
@padamstx, I forced pushed updates. Several links were added to the Swagger. |
@dubee FYI, the pull request build failed due to some compile errors in the examples code. Did you run those locally? (e.g. |
54e08eb
to
2bedcee
Compare
Signed-off-by: James Dubee <[email protected]>
@padamstx, I've updated the examples to compile and run. |
## [0.28.4](v0.28.3...v0.28.4) (2022-09-21) ### Bug Fixes * **Catalog Management:** re-gen service and tests with latest API ([#205](#205)) ([b955e79](b955e79))
🎉 This PR is included in version 0.28.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
PR summary
Re-gen of the Catalog Management SDK to get the latest API updates.
PR Checklist
Please make sure that your PR fulfills the following requirements:
Current vs new behavior
Does this PR introduce a breaking change?
Other information