-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Remove Deprecated EmailValidator #3395
Conversation
7921aa7
to
c0f3105
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am 👍 but for the record not Spree::EmailValidator
has been deprecated but the namespace-less EmailValidator
is deprecated.
Still I think we should not validate emails in core, but within extensions like solidus_auth_devise.
While I agree it's ok to remove I think a more conservative approach may be to deprecate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in favor of removing the deprecation warning but I think this will cut a whole feature off and users could be currently relying on that validation.
EmailValidator
is just proxying through ActiveSupport::Deprecation
to print a message but we are still currently using `Spree::EmailValidator', we could remove the deprecation though.
If we want to remove the validation or move it into solidus_auth_devise
I think we need a more complex plan. I think an initial good solution for this could be using a preference like Spree::Config.email_validation_class
providing a simple solution as a default, but allowing users to create extensions that replace the simple implementation with something better. What do you think?
@kennyadsl updated 👍 |
I think specs are failing due to the |
The email validator was deprecated Mar 15th 2018 almost 18 months ago as part of: solidusio@6d8fd3c The deprecation warning was part of v2.6.0rc1, and remains to this day in v2.10.0.beta1 I think it's time to remove it altogether. It's been 4 minor version upgrades now.
Rubocop warns to use `match?` instead of `=~` for performance.
@kennyadsl updated 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @JDutil!
Description
The email validator was deprecated Mar 15th 2018 almost 18 months ago
as part of: 6d8fd3c
The deprecation warning was part of v2.6.0rc1, and remains to this day
in v2.10.0.beta1 I think it's time to remove it altogether. It's been
4 minor version upgrades now.
Fixes #1794
Checklist: