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

Handle arrays in Grape::Endpoint#expose_params #742

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

fotos
Copy link
Contributor

@fotos fotos commented Feb 16, 2019

Since, in ruby-grape/grape#1863, I changed the code that made type a string, the code that generates the appropriate OpenAPI array definitions in grape-swagger broke.

In #expose_params there is a nasty trick (hack?) that removes the brackets from stringified Array classes in order to (recursively) expose the params (introduced here). The change in this PR treats arrays as first class objects.

Unfortunately I can't say I fully understand what's going on, therefore, please guide me and let me know if this is gonna introduce any regressions to existing projects.

This PR works with ruby-grape/grape#1863, thus specs might be broken for HEAD or other versions.

@coveralls
Copy link

coveralls commented Feb 16, 2019

Coverage Status

Coverage decreased (-0.1%) to 99.364% when pulling 917bd8b on fotos:fix-array-handling into e1261f8 on ruby-grape:master.

@LeFnord
Copy link
Member

LeFnord commented Mar 20, 2019

@fotos … please see my comment here #741 (comment)

and many thanks … 👍

@swistaczek
Copy link
Contributor

Hey, is there any chance that this may get merged? cc @LeFnord @fotos

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