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

Support Rails 7.2+ #101

Merged
merged 2 commits into from
Jun 5, 2024
Merged

Support Rails 7.2+ #101

merged 2 commits into from
Jun 5, 2024

Conversation

ghiculescu
Copy link
Contributor

@ghiculescu ghiculescu commented Mar 4, 2024

The schema_migration method was moved in rails/rails#51162

Red tests are due to #93

@ghiculescu
Copy link
Contributor Author

ghiculescu commented Mar 4, 2024

@casperisfine would Rails ever consider a public API for accessing this model or table name?

@casperisfine
Copy link
Contributor

would Rails ever consider a public API for accessing this model or table name?

Hum, I dunno. I was quite happy I was able to move those without breaking the public API. But I guess we could expose them on AR::Base and commit to that. But I wouldn't take this decision alone, at the very least @eileencodes would need to agree.

@eileencodes
Copy link

would Rails ever consider a public API for accessing this model or table name?

I'd be fine with schema_migration being public but not the class. When I removed the inheritance on Base it also removed the ability to do most queries, so it only responds to some methods. I don't want PRs making that class (or InternalMetadata) more robust.

@joshuay03
Copy link
Contributor

joshuay03 commented Apr 5, 2024

Not necessarily a breaking change for 7.2 but it would be great if #102 could also get included with this.

@etagwerker
Copy link
Member

@ghiculescu Looks good, thank you!

@etagwerker etagwerker merged commit 74a913a into DatabaseCleaner:main Jun 5, 2024
20 of 24 checks passed
@ghiculescu ghiculescu deleted the patch-1 branch June 5, 2024 23:05
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