fix for Deprecation warning in Rails 6.0.0 #35
Conversation
|
@mgmodell Thanks for your contribution, let me see if I can fix the build with latest rails. |
|
Thanks, @allenwq . Is this broken build due to my patch? Should I have found this on my own? |
|
Nope, it's probably my setup with rails6
…On Thu, 3 Oct 2019 at 21:46, Micah Gideon Modell ***@***.***> wrote:
Thanks, @allenwq <https://github.com/allenwq> . Is this broken build due
to my patch? Should I have found this on my own?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#35?email_source=notifications&email_token=ABGATR42AG45SE3ECRLTEFTQMXZR5A5CNFSM4I4JQZ42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAIICSQ#issuecomment-537952586>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABGATR7YCBUP7HRPVSRY523QMXZR5ANCNFSM4I4JQZ4Q>
.
|
|
@mgmodell Try to rebase and push again? |
|
I've never done this before, so I just hope I'm doing it right. I pulled your latest into my repo and then pushed back the results. Github doesn't seem to be unhappy with this so far. Was that right? |
| validates_uniqueness_of :email, allow_blank: true, case_sensitive: true, if: :will_save_change_to_email? | ||
| validates_format_of :email, with: email_regexp, allow_blank: true, if: :will_save_change_to_email? | ||
| else | ||
| validates_uniqueness_of :email, allow_blank: true, if: :email_changed? |
There was a problem hiding this comment.
Sorry I haven't worked on Rails for a while, can you tell me why case_sensitive is removed? Did rails6 make this the default?
There was a problem hiding this comment.
Also why will_save_change_to_email is changed to email_changed?, some comments might be helpful.
There was a problem hiding this comment.
It appears that case_sensitive is being deprecated and will be removed in rails 6.1. As for switching will_save_change_to_email moving to email_changed ⎯ I'm not really sure. When I went looking for an answer, I found this code in sister code devise issue 5064 and figured that if it's the direction they are taking, it's probably representative of best practices for us.
Is this sufficient for comments, or would you like me to add them in the code itself (which I find quite readable without)?
This addresses #34 .