Skip to content
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

Use a dedicated ActiveSupport::Deprecation #5583

Merged
merged 2 commits into from Jun 9, 2023

Conversation

etiennebarrie
Copy link
Contributor

Rails 7.1 will deprecate using the singleton ActiveSupport::Deprecation instance (rails/rails#47354). This defines one for the gem and uses it.

Rails 7.1 will deprecate using the singleton ActiveSupport::Deprecation
instance. This defines one for the gem and uses it.
@RailsCod3rFuture
Copy link

Hope to see these depreciation issues fixed before Rails 7.1 official release. I was playing around with MRSK. Doesn't seem plausible just yet.

@carlosantoniodasilva
Copy link
Member

I was playing around with MRSK. Doesn't seem plausible just yet.

@RailsCod3rFuture I'm not sure how this relates to the changes here? You mean the deprecations are causing you trouble with MRSK?

@RailsCod3rFuture
Copy link

RailsCod3rFuture commented Jun 5, 2023

I was playing around with MRSK. Doesn't seem plausible just yet.

@RailsCod3rFuture I'm not sure how this relates to the changes here? You mean the deprecations are causing you trouble with MRSK?

The rails server doesn't start on rails 7.1 as a result of the ActiveSupport::Deprecation instance. If someone else can check on their end for an r 7.1. app, that should confirm the symptoms that I'm experiencing. Thanks!

@carlosantoniodasilva
Copy link
Member

@RailsCod3rFuture so I'd guess this is not a Devise problem, but an MRSK problem? I mean, I see no reason why the deprecations would prevent Devise from working. (I'm not saying we aren't gonna fix it, just mentioning it shouldn't break stuff like that imo.)

@RailsCod3rFuture
Copy link

RailsCod3rFuture commented Jun 5, 2023

@RailsCod3rFuture so I'd guess this is not a Devise problem, but an MRSK problem? I mean, I see no reason why the deprecations would prevent Devise from working. (I'm not saying we aren't gonna fix it, just mentioning it shouldn't break stuff like that imo.)

This is the exception that I receive while launching the rails app for 7.1

/bin/zsh -c "bash -c 'env RBENV_VERSION=3.1.2 /usr/local/Cellar/rbenv/1.2.0/libexec/rbenv exec ruby /Users/dwood/RubymineProjects/test_app/bin/rails server -b 0.0.0.0 -p 3000 -e development'"
DEPRECATION WARNING: DeprecatedConstantAccessor.deprecate_constant without a deprecator is deprecated (called from <module:Authenticatable> at /Users/dwood/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/devise-4.8.1/lib/devise/models/authenticatable.rb:65)
/Users/dwood/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bootsnap-1.10.3/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:15:in `require': cannot load such file -- polyamorous/activerecord_7.1_ruby_2/join_association (LoadError)

@etiennebarrie
Copy link
Contributor Author

I think you're just seeing the deprecation just above your real issue with polyamorous.

@RailsCod3rFuture
Copy link

I think you're just seeing the deprecation just above your real issue with polyamorous.

Good catch. I'll probably have to reach out to the ransack team on that note....see if they have any solutions or knowledge of this issue.

@@ -521,8 +521,12 @@ def self.secure_compare(a, b)
res == 0
end

def self.deprecator
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to register the deprecator in the application deprecator. We want users to be able to silence the deprecations in their apps, or change the behavior to use notifications for example. Can you do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@rafaelfranca rafaelfranca merged commit 54a624a into heartcombo:main Jun 9, 2023
24 of 48 checks passed
carlosantoniodasilva pushed a commit that referenced this pull request Oct 10, 2023
Use a dedicated ActiveSupport::Deprecation
carlosantoniodasilva pushed a commit that referenced this pull request Oct 10, 2023
Use a dedicated ActiveSupport::Deprecation
carlosantoniodasilva pushed a commit that referenced this pull request Oct 10, 2023
Use a dedicated ActiveSupport::Deprecation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants