-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Migrate from ActiveSupport::Configurable to Dry::Configurable #2617
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
Conversation
4df6529 to
1f8c36b
Compare
dblock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some missing ... periods ;) Feel free to merge after.
UPGRADING.md
Outdated
| ### Endpoint execution simplified and `return` deprecated | ||
| #### Configuration API Migration from ActiveSupport::Configurable to Dry::Configurable | ||
|
|
||
| Grape has migrated from `ActiveSupport::Configurable` to `Dry::Configurable` for its configuration system since its [deprecated](https://github.com/rails/rails/blob/1cdd190a25e483b65f1f25bbd0f13a25d696b461/activesupport/lib/active_support/configurable.rb#L3-L7) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Grape has migrated from `ActiveSupport::Configurable` to `Dry::Configurable` for its configuration system since its [deprecated](https://github.com/rails/rails/blob/1cdd190a25e483b65f1f25bbd0f13a25d696b461/activesupport/lib/active_support/configurable.rb#L3-L7) | |
| Grape has migrated from `ActiveSupport::Configurable` to `Dry::Configurable` for its configuration system since its [deprecated](https://github.com/rails/rails/blob/1cdd190a25e483b65f1f25bbd0f13a25d696b461/activesupport/lib/active_support/configurable.rb#L3-L7). |
CHANGELOG.md
Outdated
| * [#2615](https://github.com/ruby-grape/grape/pull/2615): Remove manual toc and tod danger check - [@alexanderadam](https://github.com/alexanderadam). | ||
| * Your contribution here. | ||
| * [#2612](https://github.com/ruby-grape/grape/pull/2612): Avoid multiple mount pollution - [@alexanderadam](https://github.com/alexanderadam). | ||
| * [#2617](https://github.com/ruby-grape/grape/pull/2617): Migrate from ActiveSupport::Configurable to Dry::Configurable - [@ericproulx](https://github.com/ericproulx). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * [#2617](https://github.com/ruby-grape/grape/pull/2617): Migrate from ActiveSupport::Configurable to Dry::Configurable - [@ericproulx](https://github.com/ericproulx). | |
| * [#2617](https://github.com/ruby-grape/grape/pull/2617): Migrate from `ActiveSupport::Configurable` to `Dry::Configurable` - [@ericproulx](https://github.com/ericproulx). |
dblock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think we can preserve this to avoid a breaking change?
Grape.configure do |config|
config.param_builder = :hashie_mash
config.lint = true
end
``1f8c36b to
9a8a4c6
Compare
It's a lie from AI. It's still working. |
Migrate from ActiveSupport::Configurable to Dry::Configurable
This PR migrates Grape's configuration system from
ActiveSupport::ConfigurabletoDry::Configurable, since its deprecated (see #2616)Changes Made
Dependencies
dry-configurabledependency togrape.gemspecactive_support/configurableCore Changes
ActiveSupport::ConfigurablewithDry::Configurablein the mainGrapemoduleconfig.param_builder = :valuetosetting :param_builder, default: :valueparam_builder(default::hash_with_indifferent_access)lint(default:false)Code Changes
Benefits
active_support/configurable, reducing the overall dependency footprintDry::Configurableis more lightweight and focused specifically on configuration needsDocumentation Updates Needed
The following documentation files will need to be updated in a follow-up PR:
README.md- Update configuration examplesUPGRADING.md- Add migration guide for configuration changesTesting
api_remount_spec.rbtests that use dynamic configuration continue to work as expected