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.

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

Update drf-yasg and fix make docs #34959

Merged
merged 4 commits into from
Jun 18, 2024
Merged

Update drf-yasg and fix make docs #34959

merged 4 commits into from
Jun 18, 2024

Conversation

feanil
Copy link
Contributor

@feanil feanil commented Jun 10, 2024

  • fix: Remove the drf-yasg constraint.
  • chore: Run make upgrade-package=drf-yasg
  • build: Correct the make targets for building the docs.
  • chore: Update the openapi spec generated by drf-yasg.

@feanil feanil changed the title feanil/update drf yasg Update drf-yasg and fix make docs Jun 10, 2024
@feanil feanil mentioned this pull request Jun 10, 2024
@feanil feanil requested a review from kdmccormick June 13, 2024 12:02
@@ -2,17 +2,19 @@ swagger: '2.0'
info:
title: Open edX API
description: APIs for access to Open edX information
contact:
Copy link
Member

@kdmccormick kdmccormick Jun 18, 2024

Choose a reason for hiding this comment

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

I was trying to remember why in the world we decided to commit this generated file to source control. It looks like when I approved it originally, it was contingent on coming up with some automatic way to keep it in sync with reality, which, oops, we never did.

My gut feeling back then was that we shouldn't commit it without a syncing strategy. I still feel that way, especially now that it's part of make docs -- it'll be annoying to have make docs dirty the git state.

If you want to let this be for now, that's fine. But if you'd like to git-rm this file and put it in .gitignore, you'd have my 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use this file as a part of the docs now, it generates RST docs based on the openapi schema. I also think it's being used operationally in a few places so it's removal is a bit more complex. I generally agree with you though, I don't like how it's being managed currently. There are some improvements we can make here as we improve the edx-platform API posture.

@feanil feanil merged commit 333d228 into master Jun 18, 2024
46 checks passed
@feanil feanil deleted the feanil/update_drf_yasg branch June 18, 2024 15:24
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants