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 gem to use ActiveSupport >= 7.0 #174

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

5minpause
Copy link

@5minpause 5minpause commented Mar 22, 2023

This PR updates this gem to support the latest ActiveSupport version v7.0.4.3
In order to do this we need to fix a namespacing issue in Rails/ActiveSupport rails/rails#47733

To demonstrate that the fix in the linked PR works, I created a proof-of-concept monkeypatching the ActiveSupport::TaggedLogging class with a local copy. I will update this PR to remove the monkey-patched version once the PR to Rails is merged.

It turns out we "simply" need to load active_support to use the individual classes.

According to this comment [1] we need to load the whole of active_support
and cannot load the tagged_logging class individually.

[1] rails/rails#47733 (comment)
@skipkayhil
Copy link

It turns out we "simply" need to load active_support to use the individual classes.

To clarify a bit, require "active_support" does not load everything. However it does set up autoloads for a number of "core" classes that are used in many of the individual components (like IsolatedExecutionState):

https://github.com/rails/rails/blob/355fd5929058ee3a73032c5f62de3d8819550482/activesupport/lib/active_support.rb

@davidwinter
Copy link
Owner

What are we gaining from this? Is the older version breaking something?

@skipkayhil
Copy link

I'm not sure about the gemspec changes... adding require "active_support" is definitely the important part

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.

3 participants