fotinakis/swagger-blocks

More human friendly DSL

uesteibar opened this issue · 6 comments

Relaying on #method_missing, we can allow for a nicer DSL with the following format

parameter do
  name :tags
  key :in, :query # the `in` keyword is a reserved word in ruby
  description 'tags to filter by'
  required false
  type :array
  items do
    type :string
  end
end

There are some limitations, like the in keyword being already taken by ruby, but apart from that everything should be fine. For this in case, we could either keep it using #key or rename it to something else, but since I'd rather keep consistency with the swagger spec I prefer the first option.

This would be 100% backwards compatible.

The code is ready, so I can open the PR myself if you guys are fine with it.

Really nice! That's just what I was missing in this gem. Actually I think the in issue could be solved by using the key/value together as the method name. Something like: in_query and in_body.

@bacchir good idea!
Actually this could be implemented only in the Swagger::Blocks::Nodes::ParameterNode class, since it should only be used within that kind of node.

screen shot 2017-09-20 at 12 40 01

As you can see in the screenshot (taken from the swagger docs), the available methods should be in_query, in_path, in_cookie and in_header.

What do the maintainers think? I'd say it's a nice solution to the problem and key :in, :query would still work for whoever wants to use it.

Seems like maybe an improvement, but also I am on the fence because it's simpler to maintain without this — like you're saying above, you'd have to catch all the edge cases with reserved words, it may introduce other edge cases that would have to be documented. It also introduces a style issue where swagger keys like operationId are fine when they're key :operationId, 'foo' but would be non-ruby style if they were operationId 'foo' and probably would cause rubocop rules to fail by default. Not sure yet, what do you think?

Hi @fotinakis! Thanks for considering this change :)

For the edge cases, I accept it might be a little more difficult to maintain, but I couldn't find any more cases than in, and that can be taken care of very easily.

For rubocop, I didn't think of this CamelCase style issue to be honest. Anyway, at least in my case (and others I know) just overwrite rubocop config for the folder where they keep this docs, since rules like ClassLength, BlockLength and MethodLength don't really apply in this context.

Other trade off I can think of is that this wouldn't allow to set "key": "whatever".

What do you think? Even with this concerns, I still think this could be an improvement. Is there any channel (mailing list, slack...) where we could get a little more feedback from the community?

There is not a channel or mailing list, this is just something I maintain in my "spare" time. :) Though it's a popular library, we don't really have a list of all the users or a great way to get larger feedback like you're asking for other than just these issues.

One problem with the approach above is that swagger-blocks can also be incorporated into the API files directly, not just in a separate place, so file or folder-level rubocop configs might not be able to ignore things like operationId nicely.

I think in general I would say let's not do this right now, just to keep things simpler to maintain even though it may be more verbose. But happy to leave this open for now, discuss more, and try to get a better design for the edge cases and if it's feasible.

@fotinakis sounds good to me, we can close this and come back if it ever gets too bad or complex or we find a better way of doing this.

Thanks for considering it! 😃