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

Adds rails-7-1 support #78

Merged
merged 11 commits into from
Feb 17, 2023
Merged

Adds rails-7-1 support #78

merged 11 commits into from
Feb 17, 2023

Conversation

eis-ioki
Copy link

@eis-ioki eis-ioki commented Dec 5, 2022

In Rails 7.1 ::ActiveRecord::SchemaMigration.table_name is deprecated

@petergoldstein
Copy link
Contributor

Would love to see this merged and released in a new version. Perhaps also with Ruby 3.2 added to the CI matrix.

@petergoldstein
Copy link
Contributor

@eis-ioki You're going to want to add a Rails 7.1 gemfile and exclude the combination of Rails 7.1 with all Rubies earlier than Ruby 3.0. Plus you'll need to rebase to pick up the Ruby 3.2 addition to CI.

if Gem::Version.new("6.0.0") <= ::ActiveRecord.version
::ActiveRecord::Base.connection.schema_migration.table_name
else
::ActiveRecord::SchemaMigration.table_name
Copy link

@mathieujobin mathieujobin Feb 16, 2023

Choose a reason for hiding this comment

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

this no longer works in 7.1

please merge #82 instead

Copy link
Author

@eis-ioki eis-ioki Feb 16, 2023

Choose a reason for hiding this comment

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

That's what this PR is all about. You should use ::ActiveRecord::Base.connection.schema_migration.table_name from 6.0.0 onwards.

@eikes
Copy link
Contributor

eikes commented Feb 16, 2023

Finally got the whole matrix to pass which was difficult because of the different rubygems dependencies:

https://github.com/eikes/database_cleaner-active_record/actions/runs/4194214954

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@eis-ioki This is great. Thanks for pushing this forward! 👍🏼

@petergoldstein
Copy link
Contributor

@etagwerker Is there going to be a gem release that includes this change published in the near future? That would be very helpful.

@etagwerker
Copy link
Member

@petergoldstein https://mastodon.social/@etagwerker/109880784622263889

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.

5 participants