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

Update supported Ruby/Rails #1063

Merged
merged 8 commits into from
Dec 22, 2023
Merged

Conversation

brian-kephart
Copy link
Collaborator

This is the usual stuff.

EOL versions removed:

  • Ruby 2.7
  • Rails 6.0

Newer versions added to the build matrix:

  • Ruby 3.2
  • Rails 7.1

No changes were necessary for the new support, except to placate RuboCop after updating the target Ruby version.

Copy link
Collaborator

@texpert texpert left a comment

Choose a reason for hiding this comment

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

LGTM

I'm a little bit reluctant of dropping support of Ruby 2.7 - think there may be several apps using it yet.

… 7.1.

The issue only seems to occur when 7.1 defaults are enabled. It results in an error stating that the decorator instance must define :marshal_load.
Introducing Rails version defaults into the dummy app caused test failures related to this.
@brian-kephart
Copy link
Collaborator Author

I found a bug that affected low-level caching of decorator classes. This bug only happened when loading version 7.1 defaults, so I had to reproduce that condition in the dummy app in order to add a test for it. This made me think that the dummy app should load the defaults for whatever version of Rails is being tested against, so I added that. Using the per-version Rails defaults caused another couple of test failure that had to be fixed.

All is working now though, and the tests are now better at checking against evolving Rails defaults.

@brian-kephart
Copy link
Collaborator Author

@texpert We can support 2.7 if you want to. I thought we had decided earlier to drop support for versions when they go EOL, and that version went EOL months ago so I removed it. Supporting it wasn't causing any trouble, though.

I always figure that if someone isn't updating Ruby, they're probably not updating their gems either, but people often surprise me. 🙃

Copy link
Collaborator

@texpert texpert left a comment

Choose a reason for hiding this comment

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

LGTM, great work!

@texpert
Copy link
Collaborator

texpert commented Dec 20, 2023

@texpert We can support 2.7 if you want to. I thought we had decided earlier to drop support for versions when they go EOL, and that version went EOL months ago so I removed it. Supporting it wasn't causing any trouble, though.

I always figure that if someone isn't updating Ruby, they're probably not updating their gems either, but people often surprise me. 🙃

I'd propose to cut a 2.7.5 release before merging this branch, stating in the changelog that it is the last version with Ruby 2.7 support. We have the JS fixes in master, the SSRF mitigation, and it would be good to make a fixed final release for 2.7

And then to merge this PR and cut a new 2.8.0 release.

@brian-kephart brian-kephart merged commit 7990d69 into owen2345:master Dec 22, 2023
10 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.

2 participants