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
refactor: remove swagger route and docs annotations (DEV-1335) #2203
Conversation
✅ Linked to Story DEV-1335 · DSP-API: Remove SwaggerAPI Route |
Codecov Report
@@ Coverage Diff @@
## main #2203 +/- ##
==========================================
+ Coverage 86.64% 86.84% +0.20%
==========================================
Files 233 242 +9
Lines 28020 28177 +157
==========================================
+ Hits 24277 24471 +194
+ Misses 3743 3706 -37
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
LGTM
docs/Readme.md
Outdated
- `src/api-v1`: The DSP-API V1 (JSON) Request and Response Format documentation. Source can be found in `salsah1/src/typescript_interfaces` | ||
- `src/api-v2`: The DSP-API V2 (JSON-LD) Request and Response Format documentation. | ||
- `src/api-admin`: The DSP-API Admin (JSON) Request and Response format Swagger-based documentation. |
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.
This whole structure is not up-to-date. Could you go over the whole Readme.md
file if you touch it anyway?
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.
Although it's out of the scope of this PR I cleaned it up. There should be another task defined to clean the whole API documentation though.
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.
And I removed too much :)
Do we actually need this "test"?
test-docs-build:
name: Test docs
runs-on: ubuntu-latest
steps:
- name: Checkout source
uses: actions/checkout@v3
- name: Install requirements (pip, npm, apt)
run: |
python -m pip install --upgrade pip
pip install -r docs/requirements.txt
npm install --global typedoc
sudo apt-get install graphviz
- name: Build docs
run: make docs-build
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.
Yes, we need it. It's to make sure that the docs compile.
Issue Number: DEV-1335
Pull Request Checklist
Basic Requirements
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Does this PR change client-test-data?
Other information