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

Added API contracts #8

Merged
merged 6 commits into from
Aug 3, 2023
Merged

Added API contracts #8

merged 6 commits into from
Aug 3, 2023

Conversation

vikhyat187
Copy link
Contributor

@vikhyat187 vikhyat187 commented Jul 20, 2023 •
edited
Loading

Added the API contracts for skill tree
Closes: #4

PRD: https://docs.google.com/document/d/114ihS42_5M8kN7oM5aQ3XgHN_nxOdr17gG3C3JdeOos/edit?usp=sharing
Entities: #2

To check the Visual UI we can use https://editor.swagger.io/

@vikhyat187 vikhyat187 self-assigned this Jul 20, 2023
RitikJaiswal75
RitikJaiswal75 reviewed Jul 26, 2023
View reviewed changes
Copy link

@RitikJaiswal75 RitikJaiswal75 left a comment

Choose a reason for hiding this comment

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

image

404 and 503 should also be json response why does swagger show only text?

@RitikJaiswal75
Copy link

NIT: Could you maybe look into integrating a hosting mechanism on every update/PR it so we don't have to look at yaml/copy-paste every time to look into the contracts?

@vikhyat187
Copy link
Contributor Author

image

404 and 503 should also be json response why does swagger show only text?

Because they are in text format, let me create them in JSON format @RitikJaiswal75

@vikhyat187
Copy link
Contributor Author

vikhyat187 commented Jul 26, 2023 •
edited
Loading

NIT: Could you maybe look into integrating a hosting mechanism on every update/PR it so we don't have to look at yaml/copy-paste every time to look into the contracts?

For the code editor we have the extensions for free, but hosted ones are paid ones.

bhtibrewal
bhtibrewal previously approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@bhtibrewal bhtibrewal left a comment

Choose a reason for hiding this comment

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

LGTM for starting out, but needs auth APIs

@vikhyat187
Copy link
Contributor Author

LGTM for starting out, but needs auth APIs

Yes @kavyarishi19 is creating a doc to decide approach for authentication

@vikhyat187 vikhyat187 requested a review from bhtibrewal July 31, 2023 16:02
@vikhyat187 vikhyat187 mentioned this pull request Aug 2, 2023
Merged
1 task
@iamitprakash iamitprakash merged commit 99ecc50 into develop Aug 3, 2023
@iamitprakash iamitprakash deleted the api-contracts branch August 3, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heyrandhir heyrandhir heyrandhir approved these changes

@RitikJaiswal75 RitikJaiswal75 RitikJaiswal75 approved these changes

@iamitprakash iamitprakash iamitprakash approved these changes

@DashDeipayan DashDeipayan Awaiting requested review from DashDeipayan

@vvaibhavdesai vvaibhavdesai Awaiting requested review from vvaibhavdesai

@bhtibrewal bhtibrewal Awaiting requested review from bhtibrewal

Assignees

@vikhyat187 vikhyat187

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC] API Contracts
5 participants
@vikhyat187 @RitikJaiswal75 @iamitprakash @bhtibrewal @heyrandhir