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

Make default schemes undefined to match Swagger Spec #382

Merged
merged 1 commit into from
Apr 14, 2016

Conversation

wleeper
Copy link
Contributor

@wleeper wleeper commented Apr 14, 2016

What's this PR do, and where should I start?

This PR takes out the default value for schemes. This brings the generator in line with Swagger Spec 2.0 which indicates that schemes is an optional parameter and default behavior is to assume the same scheme as the scheme of the document.

How should this be manually tested?

Can be tested using the example (which did not work prior to this change) by running:

bundle exec rackup

From the examples directory. This will start the example up on http://localhost:9292. Drop the http://localhost:9292/swagger_doc into http://petstore.swagger.io and choose one of the methods to 'try'. It should successfully hit the back end.

Any background context you want to provide?

Ran into this problem because we run our app in development on http only and the default value of ['https', 'http'] failed to work. It also failed as noted to work with the example if started per the README.

  • Tests were updated to reflect the lack of schemes tag in the default. There was already code to override this value with custom options, those tests were not changed and still pass as expected.

* There is already code to handle the scenario where the scheme is
provided as a parameter.
@wleeper wleeper force-pushed the cr380_change_default_schemes branch from bb4be10 to 996491f Compare April 14, 2016 19:43
@krisalyssa
Copy link
Contributor

+1 for an awesome commit message.

@LeFnord
Copy link
Member

LeFnord commented Apr 14, 2016

👍 please update CHANGELOG

@LeFnord LeFnord merged commit 60a5cd0 into ruby-grape:master Apr 14, 2016
LeFnord added a commit to LeFnord/grape-swagger that referenced this pull request Feb 9, 2019
…chemes

Make default schemes undefined to match Swagger Spec → schemes are optional
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.

3 participants