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

Better deprecation warning for ActiveSupport::Deprecation.warn #47978

Closed
wants to merge 1 commit into from

Conversation

ghiculescu
Copy link
Member

ref: #47354 (comment)

It is common to see this code in apps (not in gems/engines, but in user code):

ActiveSupport::Deprecation.warn "foo"

Currently that code will emit this warning:

DEPRECATION WARNING: Calling warn on ActiveSupport::Deprecation is deprecated and will be removed from Rails (use your own Deprecation object instead)

This PR changes the warning so that it gives you more direct advice on how to fix it:

DEPRECATION WARNING: Calling warn on ActiveSupport::Deprecation is deprecated and will be removed from Rails (use ActiveSupport.deprecator.warn instead)

My understanding is that this is fine for user code, and we are not expecting users to define their own deprecators for in-app use. Given that, we should just tell the user what to do in the error message.

ref: rails#47354 (comment)

It is common to see this code in apps (not in gems/engines, but in user code):

```ruby
ActiveSupport::Deprecation.warn "foo"
```

Currently that code will emit this warning:

> DEPRECATION WARNING: Calling warn on ActiveSupport::Deprecation is deprecated and will be removed from Rails (use your own Deprecation object instead)

This PR changes the warning so that it gives you more direct advice on how to fix it:

> DEPRECATION WARNING: Calling warn on ActiveSupport::Deprecation is deprecated and will be removed from Rails (use `ActiveSupport.deprecator.warn` instead)

My understanding is that this is fine for user code, and we are not expecting users to define their own deprecators for in-app use. Given that, we should just tell the user what to do in the error message.
@skipkayhil
Copy link
Member

skipkayhil commented Apr 18, 2023

I'm not sure about this one. I think if a Rails application is complex enough that it's emitting its own deprecation warnings it should define its own Deprecator so that it can control when those warnings will raise/warn/log/etc. separately from the rest of Active Support. Otherwise we effectively have the previous behavior where all deprecations go through the same class.

Edit:

Very related: #46583 (comment)

@ghiculescu ghiculescu closed this Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants