Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: updated existing APIs to enable to generate OpenAPI documentation #157

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

feat: updated existing APIs to enable to generate OpenAPI documentation #157

wants to merge 30 commits into from

Conversation

Jagrutiti
Copy link
Member

@Jagrutiti Jagrutiti commented Dec 25, 2022 •
edited by teolemon
Loading

What

  • Added BaseModel and Body Parameters to the documentation, by updating existing APIs to enable to generate OpenAPI documentation

Closes

Change API to enable auto OpenAPI documentation #77

@Jagrutiti Jagrutiti requested a review from a team as a code owner December 25, 2022 13:02
aadarsh-ram
aadarsh-ram requested changes Dec 25, 2022
View reviewed changes
Copy link
Collaborator

@aadarsh-ram aadarsh-ram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Jagrutiti! There are some changes that I have requested, which can be easily rectified. Thanks for the contribution!

backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/api.py Outdated Show resolved Hide resolved
@teolemon teolemon added 📚 documentation Improvements or additions to documentation OpenAPI labels Dec 27, 2022
alexgarel
alexgarel reviewed Jan 2, 2023
View reviewed changes
backend/editor/models.py Outdated Show resolved Hide resolved
alexgarel
alexgarel requested changes Jan 2, 2023
View reviewed changes
Copy link
Member

@alexgarel alexgarel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR :-)

Some comments, I think we should improve a bit before merging.

BTW what is this taxonomy-editor-frontend/build directory you added ?

backend/editor/models.py Outdated Show resolved Hide resolved
backend/editor/models.py Outdated Show resolved Hide resolved
@alexgarel
Copy link
Member

BTW what is this taxonomy-editor-frontend/build directory you added ?

Sorry I misread this, it was removed, but it makes your tests fail… you should not remove the build/.empty file, it is important indeed !

alexgarel
alexgarel reviewed Jan 16, 2023
View reviewed changes
backend/editor/models.py Outdated Show resolved Hide resolved
alexgarel
alexgarel reviewed Feb 7, 2023
View reviewed changes
backend/editor/api.py Outdated Show resolved Hide resolved
backend/editor/models.py Outdated Show resolved Hide resolved
backend/editor/models.py Show resolved Hide resolved
alexgarel
alexgarel approved these changes Feb 14, 2023
View reviewed changes
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jagrutiti ok, thanks, that's a good first step.

I think we have to go further to describe the API more deeply (on the response side). Did you test everything is still working after your changes ?

backend/editor/models.py Outdated Show resolved Hide resolved
@Jagrutiti
Copy link
Member Author

I think we have to go further to describe the API more deeply (on the response side).

Do you mean adding response description as give in this documentation: https://fastapi.tiangolo.com/advanced/additional-responses/#additional-response-with-model?

Did you test everything is still working after your changes ?

All the tests cases did pass. And the APIs are working fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexgarel alexgarel alexgarel approved these changes

@aadarsh-ram aadarsh-ram Awaiting requested review from aadarsh-ram

Requested changes must be addressed to merge this pull request.

Assignees

@Jagrutiti Jagrutiti

Labels
📚 documentation Improvements or additions to documentation OpenAPI
Projects
Taxonomy-editor - CentraleSupelec 2023
Status: Backlog
📚 Document Open Food Facts
Status: Todo
🧬✎ Taxonomy Editor (General)
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Jagrutiti @alexgarel @aadarsh-ram @teolemon