Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6410 +/- ##
=======================================
Coverage 89.50% 89.50%
=======================================
Files 981 981
Lines 20479 20479
=======================================
Hits 18330 18330
Misses 2149 2149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6d5661a to
afecf0c
Compare
tvdeyen
left a comment
There was a problem hiding this comment.
Nice and thanks for taking this over. It seems we need the benchmark gem for mini_magick as a dependency for active storage in tests. Adding it to local Gemfile should be enough, since stores will be able to decide which image processor they use.
afecf0c to
af03938
Compare
af03938 to
55986c3
Compare
core/solidus_core.gemspec
Outdated
| s.add_dependency 'state_machines', ['~> 0.6', '< 0.10.0'] | ||
| s.add_dependency 'state_machines-activerecord', ['~> 0.6', '< 0.10.0'] | ||
| s.add_dependency 'omnes', '~> 0.2.2' | ||
| s.add_dependency 'benchmark', '~> 0.5' |
There was a problem hiding this comment.
I would prefer not to have this as a Solidus dependency. It highly depends on what Storage solution and Ruby version the store uses. It is not a direct dependency of us and we should always be very cautious about adding new dependencies. Have you tried to just put it in the repository main Gemfile?
There was a problem hiding this comment.
Really it should just be added as a dependency for Omnes. Requiring it doesn't depend on the storage solution or the Ruby version, although with Ruby < 4 the store would already have Benchmark. I can open a pull request on Omnes.
There was a problem hiding this comment.
Makes sense, please do add it to the Gemfile, as well as making a PR against the Omnes gem. Thank you!
i18n-tasks gets reverted to an earlier version when upgrading to Ruby 4. For some reason 0.9.0 allows Ruby 4 but 0.9.37 doesn't and reverting back to 0.9.0 introduces bugs that were fixed in subsequent versions. These versions are all quite old and we can now update to 1.1.
Until released needs solidus_auth_devise main branch for now
55986c3 to
7880a1d
Compare
Benchmark is needed by Omnes as well as one test file in legacy promotions but neither gem has it as a dependency. It is also needed by mini_magick. It is no longer loaded from the standard library as of Ruby 4.
|
Should be good to go now |
Summary
A little fix along with adding Ruby 4 to CI. Fixes an issue that was surfaced when @tvdeyen ran CI with Ruby 4 in PR #6403
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: