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

Fix boolean false parameter #1093

Merged
merged 4 commits into from
Jun 23, 2017
Merged

Fix boolean false parameter #1093

merged 4 commits into from
Jun 23, 2017

Conversation

extempl
Copy link
Contributor

@extempl extempl commented Jun 23, 2017

No description provided.

@webron
Copy link
Contributor

webron commented Jun 23, 2017

Thanks for the PR. What behavior do you see without the fix?

@extempl
Copy link
Contributor Author

extempl commented Jun 23, 2017

@webron actual behavior is missed parameter. true works properly here, so I added workaround for false only.

@extempl
Copy link
Contributor Author

extempl commented Jun 23, 2017

@webron so without fix added test will fail with http://swagger.io/v1/pets/findByStatus != http://swagger.io/v1/pets/findByStatus?status=false

@webron webron requested a review from shockey June 23, 2017 18:51
shockey
shockey approved these changes Jun 23, 2017
View reviewed changes
Copy link
Contributor

@shockey shockey left a comment

Choose a reason for hiding this comment

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

Good catch - looks like this addresses a valid edge case.

@shockey shockey merged commit 1512306 into swagger-api:master Jun 23, 2017
@shockey
Copy link
Contributor

shockey commented Jun 23, 2017

Thanks for the PR, @extempl!

This will go live in the release we run later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shockey shockey shockey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@extempl @webron @shockey