Fix db_config_with_versions arity change and backport #337
Conversation
| schema_mapped_versions = if DataMigrate::RailsHelper.rails_version_equal_to_or_higher_than_7_1 | ||
| # 7.2 removes the param for db_configs_with_versions in https://github.com/rails/rails/commit/9572fcb4a0bd5396436689a6a42613886871cd81 | ||
| # 7.1 stable backported the change in https://github.com/rails/rails/commit/c53ec4b60980036b43528829d4b0b7457f759224 | ||
| schema_mapped_versions = if Gem::Dependency.new("railties", ">= 7.1.4").match?("railties", Gem.loaded_specs["railties"].version, true) |
There was a problem hiding this comment.
and there still no support for rails < v7, because the db_configs_with_versions wasn't implemented before v7
There was a problem hiding this comment.
Right right, I see rails/rails@45eb0f3 is the commit that implemented it... Weirdly enough it didn't raise any issue when manually testing data-migrate with a rails 6.1 codebase.
I will dig into that further. Thanks for carefully considering older versions. I did not fully grasp the original issue caused by this code for rails 6.x and older versions.
There was a problem hiding this comment.
@fobiasmog is your team using any of the features coming with data-migrate v11 on rails 6.1?
looking at all our recent changes, there would be a fair bit of work to make Rails 6.1 supported again
|
I checked out the change but forgot to comment, initially. Sorry about that! I think it's safe to ship it even though it doesn't seem to cover 6.1 just yet, as 6.1 does seem to require extra effort |
Not sure how we could reliably test this one, so I've linked the two commits that forced your changes and then this change.
I've also left the railtie version check in here to make the intent clearer that this is a fix for a private rails api and not a change.
Would fix #335