Skip to content

Add default swagger filename to Api. #192

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

Merged

Conversation

ProgHaj
Copy link

@ProgHaj ProgHaj commented Aug 6, 2020

This PR suggests a fix for issue #191:

  • Add default_swagger_filename parameter to restx's api.py instead of hardcoding it.

@codecov
Copy link

codecov bot commented Aug 6, 2020

Codecov Report

Merging #192 (866028f) into master (e1ab7e3) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 866028f differs from pull request most recent head 037fe95. Consider uploading reports for the commit 037fe95 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #192      +/-   ##
==========================================
+ Coverage   96.86%   96.88%   +0.01%     
==========================================
  Files          20       20              
  Lines        2740     2725      -15     
==========================================
- Hits         2654     2640      -14     
+ Misses         86       85       -1     
Impacted Files Coverage Δ
flask_restx/api.py 96.69% <100.00%> (-0.07%) ⬇️
flask_restx/resource.py 97.95% <0.00%> (-0.12%) ⬇️
flask_restx/fields.py 96.64% <0.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1ab7e3...037fe95. Read the comment docs.

karunpoudel-chr
karunpoudel-chr previously approved these changes Sep 4, 2020
Copy link

@karunpoudel-chr karunpoudel-chr left a comment

Choose a reason for hiding this comment

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

lgtm

@ziirish
Copy link
Contributor

ziirish commented Sep 5, 2020

Hello,

Thanks for your contribution.

Although the code coverage remains unchanged would you mind adding a unit test since you introduce a new option?

@ProgHaj
Copy link
Author

ProgHaj commented Sep 29, 2020

Hello and thank you for the feedback!

I added a test for a non-default swagger name to test_swagger.py.
I wasn't quite sure if it should've been added to test_swagger or test_api.

@ProgHaj ProgHaj force-pushed the fix-multiple-docs-for-one-endpoint branch from 2bee37e to de85b9d Compare December 16, 2020 02:19
@ProgHaj ProgHaj force-pushed the fix-multiple-docs-for-one-endpoint branch from de85b9d to ad4f6f3 Compare March 3, 2021 16:46
@ProgHaj
Copy link
Author

ProgHaj commented Mar 3, 2021

Hello!

I rebased it to latest again, without any failing tests.

@ProgHaj ProgHaj force-pushed the fix-multiple-docs-for-one-endpoint branch from ad4f6f3 to 115a72a Compare May 12, 2021 09:01
@nfickas
Copy link

nfickas commented Jul 27, 2021

@ziirish Is there anything we can do to get this merge request moving?

jamesheilberg
jamesheilberg previously approved these changes Aug 13, 2021
@nfickas
Copy link

nfickas commented Aug 17, 2021

@j5awry this is something that could immensely help out my team, are you able to review this? If not, who's someone that can?

j5awry
j5awry previously approved these changes Aug 18, 2021
@j5awry
Copy link
Contributor

j5awry commented Aug 18, 2021

Sorry for letting this sit. The change and test look good. If it could be given a quick rebase, that'd be helpful. thanks!

@ProgHaj ProgHaj dismissed stale reviews from j5awry and jamesheilberg via 037fe95 August 30, 2021 09:05
@ProgHaj ProgHaj force-pushed the fix-multiple-docs-for-one-endpoint branch from 115a72a to 037fe95 Compare August 30, 2021 09:05
@ProgHaj
Copy link
Author

ProgHaj commented Aug 30, 2021

@j5awry sorry for the delay, just rebased!

@j5awry j5awry merged commit 3b93888 into python-restx:master Sep 8, 2021
@lyonsaj
Copy link

lyonsaj commented May 12, 2022

@j5awry Is there an update on this being released?

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.

8 participants