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

Add required params method #131

Merged
merged 1 commit into from May 3, 2016

Conversation

Projects
None yet
3 participants
@dtaniwaki
Contributor

dtaniwaki commented Apr 19, 2014

Hi!
I think this method is very useful for everyone who wants to ensure the query parameters. Could you consider to merge this change?

Review on Reviewable

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak May 22, 2015

Member

Hello! Thanks for your suggestion!

I'd like to consider this api before we can merge it, and that won't be until the next MINOR.

First thing that jumps out to me is the name, Params is too generic because this looks like you're going for StrongParameters c/d?

Member

zzak commented May 22, 2015

Hello! Thanks for your suggestion!

I'd like to consider this api before we can merge it, and that won't be until the next MINOR.

First thing that jumps out to me is the name, Params is too generic because this looks like you're going for StrongParameters c/d?

@zzak zzak added the feature label May 22, 2015

@zzak zzak added this to the 1.5 milestone May 22, 2015

@zzak zzak merged commit 74fcc3a into sinatra:master May 3, 2016

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@dtaniwaki dtaniwaki deleted the dtaniwaki:add-required-params branch May 3, 2016

@stjhimy

This comment has been minimized.

Show comment
Hide comment
@stjhimy

stjhimy May 3, 2016

Contributor

👍 StrongParameters
👍 RequiredParameters

Contributor

stjhimy commented May 3, 2016

👍 StrongParameters
👍 RequiredParameters

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak May 4, 2016

Member

I like RequiredParams, it matches the method name and has no conflict with other gems.

Member

zzak commented May 4, 2016

I like RequiredParams, it matches the method name and has no conflict with other gems.

zzak added a commit that referenced this pull request May 4, 2016

stjhimy added a commit to stjhimy/sinatra-contrib that referenced this pull request Jun 21, 2016

zzak added a commit to zzak/sinatra-contrib that referenced this pull request Jul 22, 2016

zzak added a commit to zzak/sinatra-contrib that referenced this pull request Jul 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment