-
Notifications
You must be signed in to change notification settings - Fork 898
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
Upgrade config to latest 5.x version for rails 7.1 compatibility #23275
Merged
Fryguy
merged 1 commit into
ManageIQ:master
from
jrafanie:upgrade-config-gem-for-backward-compatible-rails71-support
Dec 11, 2024
Merged
Upgrade config to latest 5.x version for rails 7.1 compatibility #23275
Fryguy
merged 1 commit into
ManageIQ:master
from
jrafanie:upgrade-config-gem-for-backward-compatible-rails71-support
Dec 11, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@miq-bot cross-repo-tests /all |
miq-bot
pushed a commit
to ManageIQ/manageiq-cross_repo-tests
that referenced
this pull request
Nov 20, 2024
From Pull Request: ManageIQ/manageiq#23275
jrafanie
changed the title
[WIP] Upgrade to latest 5.x version for rails 7.1 compatibility
[WIP] Upgrade config to latest 5.x version for rails 7.1 compatibility
Nov 21, 2024
jrafanie
force-pushed
the
upgrade-config-gem-for-backward-compatible-rails71-support
branch
from
November 26, 2024 16:08
00d9dc9
to
70694af
Compare
This feels like it will could bite us in more places, but I have no idea how to find it. cc @agrare |
I noticed deep_merge was behaving differently in tests and failing on 7.1. Digging in, rails 7.0 was using the gem, 7.1 was using the activesupport module: rails 7.1: ``` (byebug) {}.method(:deep_merge) #<Method: Hash(DeepMerge::DeepMergeHash)#deep_merge(source, options=...) /Users/joerafaniello/.gem/ruby/3.3.6/gems/deep_merge-1.2.2/lib/deep_merge/deep_merge_hash.rb:18> ``` rails 7.0: ``` (byebug) {}.method(:deep_merge) #<Method: Hash#deep_merge(other_hash, &block) /Users/joerafaniello/.gem/ruby/3.3.6/gems/activesupport-7.0.8.6/lib/active_support/core_ext/hash/deep_merge.rb:18> ``` Rails 7.1 changed the way their deep_merge is defined via a refactored module: https://www.github.com/rails/rails/commit/43b980368a7628fac95cc4f673e0dfbcee77c10b This was raised in: https://www.github.com/rails/rails/issues/49457 Config gem fixed this in 5.0 and subsequently fixed another compatibility issue in 5.1 https://www.github.com/rubyconfig/config/commit/759c0fe271b4b90d534217a5b6e0653d0c94ee85
jrafanie
force-pushed
the
upgrade-config-gem-for-backward-compatible-rails71-support
branch
from
December 5, 2024 14:47
70694af
to
9cf3547
Compare
jrafanie
changed the title
[WIP] Upgrade config to latest 5.x version for rails 7.1 compatibility
Upgrade config to latest 5.x version for rails 7.1 compatibility
Dec 5, 2024
Bump, thoughts on this one? I'd like to get some of these merged to continue on the path to rails 7.1 |
I'm good with merging this to master, and we can work out the kinks before branching for spassky. |
jrafanie
deleted the
upgrade-config-gem-for-backward-compatible-rails71-support
branch
December 12, 2024 16:13
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update: no longer WIP.
I noticed deep_merge was behaving differently in tests and failing on 7.1.
Digging in, rails 7.0 was using the gem, 7.1 was using the activesupport module:
rails 7.1:
rails 7.0:
Rails 7.1 changed the way their deep_merge is defined via a refactored module: https://www.github.com/rails/rails/commit/43b980368a7628fac95cc4f673e0dfbcee77c10b
This was raised in:
https://www.github.com/rails/rails/issues/49457
Config gem fixed this in 5.0 and subsequently fixed another compatibility issue in 5.1 https://www.github.com/rubyconfig/config/commit/759c0fe271b4b90d534217a5b6e0653d0c94ee85