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

Re-add thread_safe gem as explicit dependency #2456

Closed
wants to merge 1 commit into from

Conversation

mediafinger
Copy link

Purpose

Rails 7.1 does no longer require the tread_safe gem, but AMS relies on it, which leads to error messages.

Changes

Re-adding the thread_safe gem as explicit dependency.

Caveats

None known.

Related GitHub issues

Additional helpful information

I am using the same version requirements AMS used to have. In the Rails 7.1.1 app with AMS 0.10.14 I am maintaining, thread_safe 0.3.6 has been installed.

As Rails 7.1 does no longer require it, but AMS relies on it
@@ -40,7 +40,8 @@ Gem::Specification.new do |spec|
# 'i18n,
# 'tzinfo'
spec.add_development_dependency 'minitest', ['~> 5.0', '< 5.11']
# 'thread_safe'

spec.add_dependency 'thread_safe','~> 0.3', '>= 0.3.4'
Copy link
Member

Choose a reason for hiding this comment

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

can we be explicit here: add_development_dependency or add_runtime_dependency

Copy link
Author

Choose a reason for hiding this comment

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

I assumed when running tests against Rails 7.1 it will be needed as well as in production.

@mediafinger
Copy link
Author

Actually this issue might have only existed with the Release Candidate version of Rails. Or together with some older gem versions. Now with our app on Rails 7.1.1 and all gem dependencies updated, it does no longer produce error messages without having the thread_safe gem in the Gemfile.lock / bundle.

As I can not reproduce the situation anymore that lead to the AMS / thread_safe error messages, I will close this PR.

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.

None yet

2 participants