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

Add same_as validator #1850

Merged
merged 1 commit into from Dec 17, 2018
Merged

Add same_as validator #1850

merged 1 commit into from Dec 17, 2018

Conversation

glaucocustodio
Copy link
Contributor

Implements #1846.

@@ -20,6 +20,7 @@
config.include Rack::Test::Methods
config.include Spec::Support::Helpers
config.raise_errors_for_deprecations!
config.filter_run_when_matching :focus
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is out of scope of this PR, but I find it to be very useful. Can we keep?

@dblock dblock merged commit d2edd88 into ruby-grape:master Dec 17, 2018
@dblock
Copy link
Member

dblock commented Dec 17, 2018

Great stuff, merged. You can take this from here and make greater_than, less_than, etc.?

@glaucocustodio glaucocustodio deleted the same-as-validator branch December 17, 2018 20:25
@glaucocustodio
Copy link
Contributor Author

What about length: { min: 2, max: 20 } and length: { in: 2..20 }?

@dblock
Copy link
Member

dblock commented Dec 17, 2018

No, because I mean it should be relative to another value, eg.

params do
   requires :first
   requires :last, greater_than: :first
end

@dblock
Copy link
Member

dblock commented Dec 17, 2018

Related, it might make sense to extract (allow extraction of) some of these validators into separate gems at some point. We could have a whole bunch of mathematical validators, password strength validators, etc.

@glaucocustodio
Copy link
Contributor Author

Ok, I thought you were talking about length.. Gt/Lt make sense in this case.

I agree with the idea of extracting validators in another gem in the future..

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