-
Notifications
You must be signed in to change notification settings - Fork 905
Add as development dependencies gems extracted from Ruby stdlib #1512
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
Add as development dependencies gems extracted from Ruby stdlib #1512
Conversation
Specs are currently failing on `master` on my local machine when I try to run specs with `BUNDLE_GEMFILE=/home/david/code/paper_trail/gemfiles/rails_6.1.gemfile bundle exec rspec`. This isn't yet reflected in any CI builds on `master`, but I believe that it would be, if a build were to be triggered on `master`. The same error is also occurring in recent PRs that have been put up (e.g. [this PR][1] with [this failed build][2] and [this PR][3] with [this failed build][4]). [1]: paper-trail-gem#1509 [2]: https://github.com/paper-trail-gem/paper_trail/actions/runs/13835939334/job/38733099092?pr=1509 [3]: paper-trail-gem#1511 [4]: https://github.com/paper-trail-gem/paper_trail/actions/runs/13916744592/job/38941085149?pr=1511 I'm not sure why these errors have started occurring now, without any changes in `master` since [the last commit on `master`][5] that passed all checks. Maybe `bundler` changed its behavior, and we are pulling in a new/recent version of `bundler` with some relevant behavior change? [5]: paper-trail-gem@94e9c0d But, anyway, the errors seem to be caused by the extraction of various gems from the Ruby standard library into standalone gems, specifically: 1. `bigdecimal` 1. `drb` 1. `mutex_m` This change adds those gems as development dependencies of `paper_trail`. Although they are not yet causing errors, the following gems were causing a warning for me when I ran specs using Ruby 3.4.2 with `BUNDLE_GEMFILE=/home/david/code/paper_trail/gemfiles/rails_6.1.gemfile bundle exec rspec`: 1. `benchmark` 1. `logger` These are the warnings: > /home/david/code/paper_trail/spec/spec_helper.rb:64: warning: logger was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0. > You can add logger to your Gemfile or gemspec to silence this warning. > /home/david/.rbenv/versions/3.4.2/lib/ruby/gems/3.4.0/gems/activesupport-6.1.7.10/lib/active_support/dependencies.rb:299: warning: benchmark was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0. > You can add benchmark to your Gemfile or gemspec to silence this warning. > Also please contact the author of activesupport-6.1.7.10 to request adding benchmark into its gemspec. To avoid these warnings, I am also adding these two gems as development dependencies. Additionally, this change adds a `require "logger"` statement to `spec/spec_helper.rb`. This isn't needed for the other gems (I'm guessing because `activerecord` and/or `actionpack` have `require` statements for `bigdecimal`, `drb`, and `mutex_m`?), but it seems to be needed for `logger`; without it, we get an error when trying to boot specs about `NameError: uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger`.
s.add_development_dependency "benchmark", "~> 0.4.0" | ||
s.add_development_dependency "bigdecimal", "~> 3.1" | ||
s.add_development_dependency "drb", "~> 2.2" | ||
s.add_development_dependency "logger", "~> 1.6" | ||
s.add_development_dependency "mutex_m", "~> 0.3.0" |
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'm not sure of the best way to determine which version specifiers to write here.
For each gem, I looked at the latest version available (at rubygems.org).
If the latest version has a major version of 0
, then I did ~> <latest version>
.
If the latest version has a non-zero major version, then I did ~> <major.minor of latest version>
.
I have little idea if this is an optimal algorithm, and would be happy to make any changes.
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.
this seems ok to me.
going forward we might want to drop unsupported rails versions from the test matrix. per this doc 6.1 was dropped on October 1, 2024 (and 7.0 will be dropped tomorrow!).
Specs are currently failing on
master
on my local machine when I try to run specs withBUNDLE_GEMFILE=/home/david/code/paper_trail/gemfiles/rails_6.1.gemfile bundle exec rspec
. This isn't yet reflected in any CI builds onmaster
, but I believe that it would be, if a build were to be triggered onmaster
. The same error is also occurring in recent PRs that have been put up (e.g. this PR with this failed build and this PR with this failed build).I'm not sure why these errors have started occurring now, without any changes in
master
since the last commit onmaster
that passed all checks. Maybebundler
changed its behavior, and we are pulling in a new/recent version ofbundler
with some relevant behavior change?But, anyway, the errors seem to be caused by the extraction of various gems from the Ruby standard library into standalone gems, specifically:
bigdecimal
drb
mutex_m
This change adds those gems as development dependencies of
paper_trail
.Although they are not yet causing errors, the following gems were causing a warning for me when I ran specs using Ruby 3.4.2 with
BUNDLE_GEMFILE=/home/david/code/paper_trail/gemfiles/rails_6.1.gemfile bundle exec rspec
:benchmark
logger
These are the warnings:
To avoid these warnings, I am also adding these two gems as development dependencies.
Additionally, this change adds a
require "logger"
statement tospec/spec_helper.rb
. This isn't needed for the other gems (I'm guessing becauseactiverecord
and/oractionpack
haverequire
statements forbenchmark
,bigdecimal
,drb
,mutex_m
?), but it seems to be needed forlogger
; without it, we get an error when trying to boot specs aboutNameError: uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger
.I think that these changes will get specs passing on
master
again.Thank you for your contribution!
Check the following boxes:
master
(if not - rebase it).Added tests.N/A: I believe that tests will fail right now if run onmaster
. If tests pass on this branch, that is an indication of success; I don't think that new tests are warranted.Added an entry to the Changelog if the new code introduces user-observable changes.N/A: This isn't a user-observable change.