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

Improve support for JSON parameters #20

Closed
wants to merge 1 commit into from

Conversation

DurandA
Copy link
Contributor

@DurandA DurandA commented Sep 7, 2016

When using api.parameters decorator, the locations parameter is now taken into account to generate the documentation ("@api.parameters(parameters.MyParameters(many=False), locations=('json',))".

When using api.parameters decorator, the locations parameter is now taken into account to generate the documentation ("@api.parameters(parameters.MyParameters(many=False), locations=('json',))".
@coveralls
Copy link

coveralls commented Sep 7, 2016

Coverage Status

Coverage remained the same at 89.448% when pulling 0840cb0 on DurandA:json-params into 054b717 on frol:master.

@@ -70,6 +70,8 @@ def decorator(func):
_locations = ('json', )
else:
_locations = locations
if locations is not None:
Copy link
Owner

Choose a reason for hiding this comment

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

I think we need to check _locations (with underscore) here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intent was to set parameters context only if locations was user-specified.

@frol
Copy link
Owner

frol commented Sep 9, 2016

I'm sorry for the delay, I have been out of town. Once we resolve the above comment, I am happy to merge this PR.

@frol
Copy link
Owner

frol commented Sep 10, 2016

I have cherry-picked your PR and merged it into master with the according fixes.

Thank you!

@DurandA
Copy link
Contributor Author

DurandA commented Sep 12, 2016

Thank you!

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.

None yet

3 participants