-
Notifications
You must be signed in to change notification settings - Fork 230
Improve Swagger annotations for PackageController and ProjectController #3540
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
base: main
Are you sure you want to change the base?
Improve Swagger annotations for PackageController and ProjectController #3540
Conversation
heliocastro
left a comment
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.
Good to go
|
@RajvardhanT7747 You have to fix some issues.. |
Thanks for pointing this out. |
65f8fb9 to
c59e99a
Compare
|
hey @heliocastro , I’ve resolved the conflicts, updated the commit author to match my signed ECA, and force-pushed the changes. |
db31968 to
0612fe3
Compare
|
Hey @heliocastro , Sorry for taking a bit more of your time. While addressing the CI and ECA checks, I had to make a few additional changes and resolve some conflicts, so I’ve updated the PR again. Could you please take another look when you get a chance and let me know if everything looks fine now? I really appreciate your patience and guidance — I was running into quite a few first-time contributor issues here. Thanks a lot! |
|
Hi team , @heliocastro @amritkv, The CI is failing due to a CouchDB The error: "chttpd_auth_cache changes listener died because the _users database does not exist" Could you please check the CI setup? My changes don't modify any database-related code. Thank you! |
|
Hey @heliocastro , My PR is failing in CI with a CouchDB _users database error. Since I only added Swagger annotations (no database changes), this seems like a CI infrastructure issue. Should I: What would you recommend? |
e4169a7 to
2e6cab2
Compare
|
Hi @heliocastro , Thank you so much for your support . The ECA issue is resolved now and that check is passing. Could you please let me know how you’d prefer I proceed here , should I investigate and fix the CI failure on my side, or is this something a maintainer usually handles ? Happy to take care of it either way. |
GMishx
left a comment
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.
Hey @RajvardhanT7747 , the documentation changes you have done are acceptable. We just need to be careful about what possible error the endpoint can throw.
In addition, your PR is very difficult to review as it contains lot's of unnecessary changes (mostly formatting). Please do not change lines which are irrelevant for this PR, it makes it very hard to find the actual changed lines and connect the dots.
|
Also, your code is not compiling in the test cases. Please check. |
This PR improves Swagger/OpenAPI documentation for two REST controllers:
The goal is to document all possible HTTP response codes properly, as currently most APIs only show 200 in Swagger.
Changes done:
Important notes:
Testing:
Since there are many controllers in the codebase, I have started with these two files.
If this approach looks correct, I can apply the same pattern to the remaining controllers.
Please let me know if any changes or improvements are needed.