-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Removed deprecations warning output for Devise::Models::Authenticatable::BLACKLIST_FOR_SERIALIZATION
#5598
Removed deprecations warning output for Devise::Models::Authenticatable::BLACKLIST_FOR_SERIALIZATION
#5598
Conversation
…ble::BLACKLIST_FOR_SERIALIZATION`
df57366
to
e524a3d
Compare
Could we get a new release for this? |
I can look into it, but is it causing any issue I'm not aware of? It is technically a breaking change which means a patch bump might not be enough for this, so I've been holding off until deciding what to do next. |
It means you get a deprecation warning when you boot an app running Rails 7.1 (alpha). So not a problem, just a little nuisance. |
@ghiculescu that's interesting, my understanding is that the deprecation shouldn't trigger unless the constant was accessed directly? I don't see it on a v7.0 app, I'll have to test with main / 7.1 alpha. |
This is the warning:
So my bad, it's the absence of #5583 that is causing it. |
@ghiculescu @carlosantoniodasilva Hi. This happens by calling This is because from The solution is to change I think it will probably be removed by the next major update. |
@ghiculescu gotcha, makes sense. That may or may not be easier to handle (in terms of a new release that's backwards compatible) @soartec-lab thanks. |
OK, If there is no prospect of responding to the PR below, I will create a PR to update this file to the latest version. What do you think? |
@soartec-lab if all we need is to update |
I have the same opinion. It looks like there was an update missing, so I'll create a new PR and fix it. |
I corrected this issue with the PR below. Could you review? |
This deprecation warning worked for us, but it has served its purpose.
It was added over 2 years ago now, isn't that long enough to trigger a deprecation warning?
https://github.com/heartcombo/devise/blob/main/CHANGELOG.md#480---2021-04-29
Now that I've given it enough time, I've removed this as I think it's noise for many people who don't see
Devise::Models::Authenticatable::BLACKLIST_FOR_SERIALIZATION
.However,
Devise::DeprecatedConstantAccessor
, which was added in the same commit, is not referenced anywhere else, but has not been removed as it may be used in the future. Please let me know so I can delete it if necessary.These source codes were added in the PR below:
0c2cab7