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.

Sign up for GitHub

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

Jump to bottom

Fix db_config_with_versions arity change and backport #337

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

Fix db_config_with_versions arity change and backport #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ Then, in the Engine's `db/data` folder, you can add data migrations and run them
Run tests for a specific version of Rails

```

bundle exec appraisal rails-6.1 rspec
bundle exec appraisal rails-7.0 rspec
bundle exec appraisal rails-7.1 rspec
Expand Down
4 changes: 3 additions & 1 deletion lib/data_migrate/database_tasks.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ def self.migrate_with_data

ActiveRecord::Migration.verbose = ENV["VERBOSE"] ? ENV["VERBOSE"] == "true" : true

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)
Copy link

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 •
edited
Loading

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 •
edited
Loading

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

ActiveRecord::Tasks::DatabaseTasks.db_configs_with_versions
else
db_configs = ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env)
Expand Down
1 change: 1 addition & 0 deletions lib/data_migrate/rails_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def rails_version_equal_to_or_higher_than_7_2

@equal_to_or_higher_than_7_2 = Gem::Dependency.new("railties", ">= 7.2.0.alpha").match?("railties", Gem.loaded_specs["railties"].version, true)
end

def rails_version_equal_to_or_higher_than_7_1
return @equal_to_or_higher_than_7_1 if defined?(@equal_to_or_higher_than_7_1)

Expand Down