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

remove explicit require for thread_safe #2395

Merged
merged 2 commits into from
Oct 16, 2020
Merged

remove explicit require for thread_safe #2395

merged 2 commits into from
Oct 16, 2020

Conversation

ritikesh
Copy link

@ritikesh ritikesh commented Oct 12, 2020

In Rails master, tzinfo has been updated to 2.x, which no longer depends on the thread_safe gem and has moved to concurrent-ruby instead. Thereby, unless you're explicitly adding thread_safe to your Gemfile, active_model_serializers throws up a cannot require thread_safe. #2369 has already replaced ThreadSafe::Cache with Concurrent::Map. This require should no longer be required!(😅 )

@bf4
Copy link
Member

bf4 commented Oct 12, 2020

Looks good. Wanna help fix CI, too?

/home/travis/build/rails-api/active_model_serializers/vendor/bundle/ruby/2.2.0/gems/pry-byebug-3.6.0/lib/pry-byebug/commands/exit_all.rb:5:in `<module:PryByebug>': uninitialized constant Pry::Command::ExitAll (NameError)
	from /home/travis/build/rails-api/active_model_serializers/vendor/bundle/ruby/2.2.0/gems/pry-byebug-3.6.0/lib/pry-byebug/commands/exit_all.rb:1:in `<top (required)>'

@ritikesh
Copy link
Author

ritikesh commented Oct 12, 2020

Hi @bf4, thanks for responding. Have tried fixing by removing the redundant gem itself. I believe pry & byebug gems should be more than enough.

@ritikesh
Copy link
Author

Hi @bf4 - looks like the CI is still failing on this, but don't think it's related to my changes.

@bf4
Copy link
Member

bf4 commented Oct 16, 2020

@ritikesh probably not related to your changes. I was just wondering if you could help fix it :)

@connorshea
Copy link

This is causing problems for upgrading to Rails 6.1 (see above rails issue), could we get a new release with this fix?

@ritikesh
Copy link
Author

ritikesh commented Dec 3, 2020

#2398

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