sxross / MotionModel

Simple Model and Validation Mixins for RubyMotion
MIT License
192 stars 67 forks source link

The README is misleading on some validation syntax #106

Closed gaelian closed 10 years ago

gaelian commented 10 years ago

In the README for this project, examples of validation syntax include:

validate :field_name, :presence => true
validate :field_name, :length => 5..8 # specify a range
validate :field_name, :email
validate :field_name, :format

I note that validate :field_name, :email actually doesn't work. After looking at the relevant source and spec, I gather that one actually needs to use validate :field_name, :email => true in order for things to work as expected.

So my question is two-fold: should one be able to write validate :field_name, :email as the README states, or should one need to write validate :field_name, :email => true as the spec states? In the former case, I'm happy to make some modifications to the Validatable#validate_one method (plus whatever spec changes are required) so that you can pass in a non-Hash value for the validation argument of that method. In the latter case, I'm happy to send over a PR updating the README, unless you feel that just updating the README yourself is less hassle.

sxross commented 10 years ago

It would be great if you could write a couple of specs:

describe "email validation syntax" do it "validates correctly with the hash syntax for email" do end

it "does not validate correctly unless hash syntax is used for email do end end

That and a README clarification would be incredibly useful so we have both documentation clarification and a regression test.

Thanks

On Feb 23, 2014, at 3:26 AM, Gaelian Ditchburn notifications@github.com wrote:

In the README for this project, examples of validation syntax include:

validate :field_name, :presence => true validate :field_name, :length => 5..8 # specify a range validate :field_name, :email validate :field_name, :format I note that validate :field_name, :email actually doesn't work. After looking at the relevant source and spec, I gather that one actually needs to use validate :field_name, :email => true in order for things to work as expected.

So my question is two-fold: should one be able to write validate :field_name, :email as the README states, or should one need to write validate :field_name, :email => true as the spec states? In the former case, I'm happy to make some modifications to the Validatable#validate_one method (plus whatever spec changes are required) so that you can pass in a non-Hash value for the validation argument of that method. In the latter case, I'm happy to send over a PR updating the README, unless you feel that just updating the README yourself is less hassle.

— Reply to this email directly or view it on GitHub.

gaelian commented 10 years ago

OK, I can sort something like that out. Just to make sure I understand you correctly, as things currently stand, the spec is correct on this point and the README is not, yes?

sxross commented 10 years ago

That would seem correct

On Feb 23, 2014, at 11:00 PM, Gaelian Ditchburn notifications@github.com wrote:

OK, I can sort something like that out. Just to make sure I understand you correctly, as things currently stand, the spec is correct on this point and the README is not, yes?

— Reply to this email directly or view it on GitHub.