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

Rails 7.2 update #312

Merged
merged 6 commits into from
Aug 15, 2024
Merged

Rails 7.2 update #312

merged 6 commits into from
Aug 15, 2024

Conversation

vprigent
Copy link
Collaborator

@vprigent vprigent commented Jun 4, 2024

This brings support for Rails 7.2 connection configuration changes.

CI fails because Ruby 3.0 isn't compatible with Rails 7.2
activesupport-7.2.0.beta1 requires ruby version >= 3.1.0, which is incompatible
I'd be keen to learn of a way around that if there is one?

Also noted, Ruby 3.0 is EOL'd, so a possible solution might be to drop it?

@@ -54,7 +63,9 @@ def data_schema_delete_version(version)
end

def data_schema_migration
if rails_version_equal_to_or_higher_than_7_1
if rails_version_equal_to_or_higher_than_7_2
DataMigrate::DataSchemaMigration.new(ActiveRecord::Tasks::DatabaseTasks.migration_connection.pool)
Copy link
Contributor

Choose a reason for hiding this comment

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

Base on Rails' code, would it be fair to use #migration_connection_pool instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call, turns out ActiveRecord::Base also has connection_pool over connection.pool 👍

Changelog.md Outdated
@@ -1,5 +1,8 @@
# Changelog

## 9.5.0
- Support Rails 7.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add your PR change link here also?

@capripot
Copy link
Contributor

capripot commented Jun 9, 2024

I agree, we should follow Rails' cadence and drop Ruby 3.0 too. Should we go to version 10 then?

@vprigent
Copy link
Collaborator Author

I agree, we should follow Rails' cadence and drop Ruby 3.0 too. Should we go to version 10 then?

Good point, I'll leave that decision to one of the contributors. perhaps @ngan would be able to review and cut a new version if they deem relevant?

@Morozzzko
Copy link
Collaborator

Rails 7.2 came out and now the lack of this fix is causing issues. Would be nice to get just about anything at this point

@bruno-costanzo
Copy link
Collaborator

Why is not possible to merge this?

@ilyakatz
Copy link
Owner

Hey folks, previous maintainers are no longer able to main the gem and I've been out of Rails ecosystem for many years now. So looking for someone willing to take over maintaining this gem .

In the meantime, @vprigent , thanks for taking this on. Since Ruby 3.0 is EOL, it can be removed from here

and hopefully that will pass the build

after the build passes, I can deploy rc release for this change

@Morozzzko
Copy link
Collaborator

I'm somewhat invested into having up-to-date tools, so I can probably help with handling updates, deprecations and PRs which solve some of the obvious issues. Can't promise anything proactive in terms of evolution or making decision that'd affect the way this gem works, though.

@bruno-costanzo
Copy link
Collaborator

I can help also!

@lorennav
Copy link

I am available to help as well

@ilyakatz
Copy link
Owner

thanks folks, please send me an email @gmail and i can add you to maintainer group and slack channel for maintainers (not much action there, but easier to coordinate)

@vprigent
Copy link
Collaborator Author

Hi folks, good to see there's some attention and motivation to maintain the project.

I've updated the CI config to reflect the Ruby 3.0 deprecation. I believe this is ready to be merged.

@ngan
Copy link
Collaborator

ngan commented Aug 15, 2024

Mind updating your branch?

@capripot
Copy link
Contributor

✅ I've been using this branch successfully in Rails 7.2 beta2

tasks/databases.rake Outdated Show resolved Hide resolved
@ngan ngan merged commit 9a7c6c5 into ilyakatz:main Aug 15, 2024
13 checks passed
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.

7 participants