-
Notifications
You must be signed in to change notification settings - Fork 64
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 CI and Rails 7.1 compatibility on main #107
Conversation
Stable builds: - Test against Ruby 3.3 stable - Test against Rails 7.1 stable - Test against Rails 7.2 beta Experimental builds: - Test against Rails edge (previously: 7.1, ref: DatabaseCleaner#106) - Test against Ruby head Fixes: - Fix sqlite3 dependency on old Rails versions Updates: - Update GitHub actions Readability: - Sort and group inclusion and exclusions in CI matrix
Also: - Invert the logic to make the conditional more readable - Add "alpha" to version to include pre-releases ActiveRecord: - 7.2: `Base.connection_pool.schema_migration.table_name` - 7.1, 7.0, 6.x: `Base.connection.schema_migration.table_name` - 5.x: `SchemaMigration.table_name` Close DatabaseCleaner#106
Removes dependency on version
if Gem::Version.new("7.1.0") < ::ActiveRecord.version | ||
if ::ActiveRecord::Base.connection_pool.respond_to?(:schema_migration) # Rails >= 7.2 | ||
::ActiveRecord::Base.connection_pool.schema_migration.table_name | ||
elsif Gem::Version.new("6.0.0") <= ::ActiveRecord.version | ||
elsif ::ActiveRecord::Base.connection.respond_to?(:schema_migration) # Rails >= 6.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opinionated change in a separate commit, feel free to remove the last commit to use a version-based comparison.
As a side node, these changes are introduced in alphas, betas, and even RCs, so I prefer respond_to?
instead of relying on the version
This fixes a bug that's currently affecting people. It would be fantastic if this were released. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tagliala Looks good, thank you! ❤️
Improve CI
Stable builds:
Experimental builds:
Fixes:
Updates:
Readability:
Fix Rails 7.1 compatibility
Also:
ActiveRecord:
Base.connection_pool.schema_migration.table_name
Base.connection.schema_migration.table_name
SchemaMigration.table_name
Use
respond_to?
to determine the schema migration table nameClose #106