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

Replace RequestStore with ActiveSupport::CurrentAttributes #702

Merged

Conversation

punkisdead
Copy link

Based on the work done here: #673 by @the-spectator

@punkisdead punkisdead marked this pull request as ready for review April 2, 2024 14:08
@punkisdead
Copy link
Author

punkisdead commented Apr 2, 2024

Since ActiveSupport::CurrentAttributes was introduced in Rails 5.2 Rails 5.2 Release I had to increase the minimum required version of activesupport and activerecord to 5.2.

I'm not entirely sure why the Fiber spec would pass on Rails 7.0 and not previous versions, but I'm not entirely sure we need that spec in this gem? ¯\_(ツ)_/¯

Even in current Rails, the default isolation level is set to :thread according to Configuring Rails

@danielmorrison
Copy link
Member

👋
I think this a reasonable time to cut support for those old Rails versions. Up for cleaning up the build matrix? Otherwise I will in a bit.

@punkisdead
Copy link
Author

I can take a look at it...

@danielmorrison
Copy link
Member

I can't remember offhand why we added the fiber spec. Probably sanity check for… something. 🤷

@punkisdead
Copy link
Author

I can't remember offhand why we added the fiber spec. Probably sanity check for… something. 🤷

It was added by @the-spectator in his PR.

@punkisdead
Copy link
Author

giphy

@danielmorrison danielmorrison merged commit c721d5d into collectiveidea:main Apr 2, 2024
58 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