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

Order of header parameters #448

Merged
merged 4 commits into from
Jun 7, 2016

Conversation

anakinj
Copy link
Contributor

@anakinj anakinj commented Jun 6, 2016

Noticed that the order of the parameters had changed at some point, nowadays the header parameters are added to the end of the parameter array.

That results in possible headers are located at the end of the list in Swagger UI.

What do you think of always having the possible header parameters at the beginning of the list?

@dblock
Copy link
Member

dblock commented Jun 6, 2016

I don't see why not. I would spec it as well.

@anakinj
Copy link
Contributor Author

anakinj commented Jun 6, 2016

Added test and changelog entry.

@@ -6,7 +6,7 @@

#### Fixes

* Your contribution here.
* [#448](https://github.com/ruby-grape/grape-swagger/pull/448): Header parameters are now prepended to the parameter list - [@anakinj](https://github.com/anakinj).
Copy link
Member

Choose a reason for hiding this comment

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

please move the entry to features, cause it is not a bug 😉

@LeFnord
Copy link
Member

LeFnord commented Jun 6, 2016

👍 … after addressing comment

@LeFnord LeFnord merged commit 893e813 into ruby-grape:master Jun 7, 2016
LeFnord pushed a commit to LeFnord/grape-swagger that referenced this pull request Feb 9, 2019
* Prepend the header params instead of appending them

* Added test for order of the parameters, headers first

* Updated CHANGELOG

* Changelog entry from fixes to features
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.

4 participants