-
-
Notifications
You must be signed in to change notification settings - Fork 233
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
Avoid defining Hash#deep_merge
and #deep_merge!
#342
Avoid defining Hash#deep_merge
and #deep_merge!
#342
Conversation
@cjlarose what do you think? |
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.
Could you please update the CHANGELOG as well?
`config` uses `DeepMerge.deep_merge!` instead of `Hash#deep_merge!`, so monkey patching `Hash` is unnecessary. Furthermore, DeepMerge's `Hash` monkey patch is not compatible with Rails 7.1 (see [rails#49457][]). This commit changes `require 'deep_merge'` to `require 'deep_merge/core'` so that DeepMerge's `Hash` monkey patch is no longer loaded. Users who rely on the monkey patch can load it manually via `require 'deep_merge/deep_merge_hash'`. Closes rubyconfig#314. [rails#49457]: rails/rails#49457
6f335b1
to
4a0b7a5
Compare
Done! |
@jonathanhefner I extended our test suite for Rails 7.x in #344, but test fails with a |
#344 got finally merged so we can verify this PR agains Rails 7.x. Let's see if it works. |
Looks it works! Merging and releasing... |
[email protected] is compatible with Rails 7.1 rubyconfig/config#342
[email protected] is compatible with Rails 7.1 rubyconfig/config#342
[email protected] is compatible with Rails 7.1 rubyconfig/config#342
config
usesDeepMerge.deep_merge!
instead ofHash#deep_merge!
, so monkey patchingHash
is unnecessary. Furthermore, DeepMerge'sHash
monkey patch is not compatible with Rails 7.1 (see rails#49457).This commit changes
require 'deep_merge'
torequire 'deep_merge/core'
so that DeepMerge'sHash
monkey patch is no longer loaded. Users who rely on the monkey patch can load it manually viarequire 'deep_merge/deep_merge_hash'
.Closes #314.