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

Fix incorrect deprecations #1357

Merged
merged 1 commit into from
Apr 10, 2016
Merged

Fix incorrect deprecations #1357

merged 1 commit into from
Apr 10, 2016

Conversation

namusyaka
Copy link
Contributor

ref #1344

@namusyaka namusyaka changed the title Fix incorrect deprecations [WIP] Fix incorrect deprecations Apr 9, 2016
@@ -56,6 +57,15 @@ def route_path
pattern.path
end

def hidden
Copy link
Member

Choose a reason for hiding this comment

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

Are we really just trying to make grape-swagger happy here? The whole hidden thing is not a Grape feature, just happens to be an option that grape-swagger "extends". Maybe it's better to expose options instead and let grape-swagger deal with it?

@namusyaka
Copy link
Contributor Author

@dblock Thanks for your opinion. I've just tried to reflect it to my changes.

@dblock
Copy link
Member

dblock commented Apr 9, 2016

I am happy with this as is, squashed and CHANGELOGed.

@dblock
Copy link
Member

dblock commented Apr 9, 2016

I also think the code needs comments of what settings vs. options is.

@namusyaka
Copy link
Contributor Author

@dblock Could you review this updates? I also squashed :)

@namusyaka namusyaka changed the title [WIP] Fix incorrect deprecations Fix incorrect deprecations Apr 9, 2016
@dblock
Copy link
Member

dblock commented Apr 9, 2016

Looks good to me, feel free to merge on green if I don't get to it.

@dblock dblock merged commit 70e67d3 into master Apr 10, 2016
@namusyaka namusyaka deleted the fix-incorrect-deprecations branch September 14, 2016 04:08
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.

None yet

2 participants