Deprecate ActiveSupport::Configurable#53970
Conversation
7052681 to
83d5a2e
Compare
This proposal is based on a [code review comment][]: > I think we should deprecate and remove this module. It is only used > once inside the framework, in a place that perhaps don't even need it > anymore and I don't think it adds much for our users. They would be > better served with their own config object that is just a hash. To achieve this outcome, replace the only internal occurrence of `ActiveSupport::Configurable` with a [class_attribute][]. Similarly, replace [config_accessor][] calls with class- and instance-level calls to [delegate][] for the readers and writers. [code review comment]: rails#53796 (comment) [class_attribute]: https://edgeapi.rubyonrails.org/classes/Class.html#method-i-class_attribute [config_accessor]: https://edgeapi.rubyonrails.org/classes/ActiveSupport/Configurable/ClassMethods.html#method-i-config_accessor [delegate]: https://edgeapi.rubyonrails.org/classes/Module.html#method-i-delegate
83d5a2e to
da2fbb2
Compare
Similar to rails/rails#53970, replicates its functionality inside `ViewComponent::Configurable`.
|
I feel like just mentioning |
This has been deprecated in Rails 8.1, and although it is still available until 8.2, replacing its usage now removes a deprecation warning. Follows an approach similar to rails/rails#53970.
For context see rails/rails#53970. We're also moving away from a top level module to class and use a version constraint for the supported rails versions only.
For context see rails/rails#53970. We're also moving away from a top level module to class and use a version constraint for the supported rails versions only.
For context see rails/rails#53970. We're also moving away from a top level module to class and use a version constraint for the supported rails versions only.
|
I don't think deprecating this module was a fantastic idea; it provided a really nice interface to implement a very common pattern, and the starting point:
seems to overlook the 7.5k files yielded by a search for Besides the rather terse message mentioned in the previous comment, the few PRs I've looked that address the deprecation end up with code that is arguably harder to read/understand. Could we reconsider the deletion/deprecation of |
Motivation / Background
This proposal is based on a code review comment:
Detail
To achieve this outcome, replace the only internal occurrence of
ActiveSupport::Configurablewith a class_attribute. Similarly, replace config_accessor calls with class- and instance-level calls to delegate for the readers and writers.Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]