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

Add support for Rails v8.x #895

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Conversation

benedikt
Copy link
Contributor

@benedikt benedikt commented Nov 8, 2024

This pull request relaxes the dependency requirements of flipper-active_record and flipper-active_support_cache_store to support Rails v8.x. This fixes #894.

It also adds Rails v8 and Rails v7.2 to the CI matrix.

Unfortunately, a lot of tests are currently failing, but the failures seem to be in line with the current failures on main.

@benedikt benedikt force-pushed the rails-8 branch 2 times, most recently from 3597e43 to 68ea4ef Compare November 8, 2024 11:11
@stevehill1981
Copy link

Aside from the OpenStruct failures, the others are all related to a change in rackup. Puma needs an update to work with that, but in the absence of a new release post-6.4.3, the only reliable fix I've found is pinning rackup to 1.0.0, prior to the changes in 1.0.1.

@benedikt
Copy link
Contributor Author

benedikt commented Nov 8, 2024

I've opened a new pull request (#896) to remove OpenStruct from the tests.

My gut feeling is that the best way to fix the rackup issue is to wait for a new release from puma.

I've combined the changes for Rails 8, with the removal of OpenStruct and pinning rackup to 1.0.0 at benedikt/flipper@0fa7b01 and the tests are all passing.

@stevehill1981
Copy link

Makes sense to me, @benedikt - hopefully we'll see a new puma release soon, from what I can tell the fix was made a couple of weeks ago (in puma/puma#3532) but there's no new version yet.

@leoarnold
Copy link

@benedikt What about pinning rackup to ~> 2?

@kmcphillips
Copy link

Have you considered dropping support for old versions and bumping the version here? You've got 'activerecord', '>= 4.2' which reached end of life 8 years ago, and you've got down to 5.2 in CI which reached end of life 3 years ago.

Even 7.1 has ended active support and 7.0 is almost beyond security support. You make your life easier by dropping those versions here and having a smaller support matrix.

Either way, thank you for doing this update. It's blocking some of our upgrades but it appears I'm mostly too late to help. 🙏

@larouxn
Copy link

larouxn commented Nov 18, 2024

👋 Inspired by #895 (comment) and our otherwise fully up-to-date service being blocked from upgrading to Rails 8 until this PR is fixed up and merged, I took a look at what cleaning up all EOL Ruby and Rails usage would entail. Kind of taking this PR to its logical conclusion assuming one could remove all EOL stuff.

I don't expect such a cleanup would be merged as Flipper likely wants to target and support some older versions for existing users/customers. That said, I think it can serve as a reference for potential future cleanup.

@jnunemaker
Copy link
Collaborator

@larouxn appreciate that but I'm not going to remove the EOL stuff. There are still people using them (right or wrong) so if I can figure out the CI stuff today I'll leave stuff around. I try very hard to remain backwards compatible. If you need to update to rails 8, you can lock to main or the latest SHA in main and you should have no issues until a new release is out.

@benedikt
Copy link
Contributor Author

@jnunemaker The main branch of my fork is currently passing CI with minimal changes. Feel free to pick whatever you need to make CI pass on the main repo.

@jnunemaker jnunemaker merged commit ebdc458 into flippercloud:main Nov 18, 2024
33 of 41 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.

No Rails 8 support
6 participants