-
Notifications
You must be signed in to change notification settings - Fork 109
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
Namespace EmailValidator under the ValidEmail2 namespace. #79
Conversation
This is a breaking change. The invocation changes from: validates :email, email: true to validates :email, 'valid_email_2/email': true This solves micke#46
All support for Ruby 2.1 has ended, so it (along with 2.0) should probably be removed from the CI checks. Those are the only ones failing. |
Hi Fred. Thanks for working on this. Wow, i had no idea you could namespace validations like this in rails. I have two problems with this though; First, as you already pointed out this is a breaking change. This will break peoples code which isn't that bad if we release it as It's kinda ugly imo, only because of the name of the gem One solution that would make it backwards compatible and available namespaced at the same time is to include class EmailValidator < ValidEmail2::EmailValidator
end in This still means that we occupy the If people complain that the gem is occupying the What do you think? |
Hi @lisinge ! I think your inheritance suggestion is a good idea. I'll go ahead and implement that today. And then it'll be up to you whether to release as a 2.0 or less. I think people upgrading major versions should know that they contain breaking changes, so it's not a big deal IMO. There is definitely a need for this due to the conflict with Devise... Led to strange behaviors in my own app for sure. |
For now, the invocation can continue to be the same: validates :email, email: true as well as: validates :email, 'valid_email_2/email': true
Great work! Thanks for your help! |
v2.0.0 is now released with this PR, i also added a deprecation warning with link to upgrade instructions in 3376119. Thanks again! |
Yay! Thanks for the release! |
This is a breaking change. The invocation changes from:
to
This solves #46