Skip to content
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

Fix db_config_with_versions arity change and backport #337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vprigent
Copy link
Collaborator

@vprigent vprigent commented Sep 9, 2024

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

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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and there still no support for rails < v7, because the db_configs_with_versions wasn't implemented before v7

Copy link
Collaborator Author

@vprigent vprigent Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@vprigent vprigent Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArgumentError: db_configs_with_versions in Rails 7.1.0 - 7.1.3.4
2 participants