-
Notifications
You must be signed in to change notification settings - Fork 173
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 dependencies used in development and on CI #485
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for this 👍
gem.add_development_dependency('spy') | ||
gem.add_development_dependency('minitest', '>= 2.11.0') | ||
|
||
if RUBY_PLATFORM == 'java' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this changes behaviour when someone tries to run this gem on JRuby, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just raising before. However, there is no need to add that constraint here, since Identity Cache doesn't itself require MRI ruby, even though we end up testing with MRI specific backends.
gem 'byebug', platform: :mri | ||
gem 'stackprof', platform: :mri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we specifying MRI only (platform: :mri
) for these gems? I assume it is related to native extensions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, these are native extensions, so we can just not install them when not using MRI.
@@ -3,5 +3,8 @@ gemspec path: '..' | |||
|
|||
gem 'activerecord', github: 'rails/rails' | |||
gem 'activesupport', github: 'rails/rails' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gem 'activesupport', github: 'rails/rails' | |
gem 'activesupport', github: 'rails/rails', branch: 'main' |
Co-authored-by: Rafael França <rafael.franca@shopify.com>
Problem
I noticed that we were using some old development dependencies (e.g. mocha 0.14.0 is from 2013), weren't actually testing the latest version of optional runtime dependencies (e.g. memcached_store) and weren't testing the latest version of ruby (3) on CI.
Solution
I used the gemspec to specify the common development and CI dependencies and pinned these dependencies to a major version.
I've added a version constraint for optional runtime dependencies in gemfiles/Gemfile.min-supported and removed these constraints from gemfiles/Gemfile.latest-release so that we can test old and new versions of these gems.
The above allowed ruby 3 to be tested without issues, so I updated CI to use ruby 3 as the latest ruby version.