-
Notifications
You must be signed in to change notification settings - Fork 46
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
Fix CI #534
Conversation
d05f405
to
fc9f1ec
Compare
`ActiveSupport::LoggerThreadSafeLevel` depends on `logger`, but doesn't require it explicitly prior to 7.1. In the real application, the `logger` is likely required by other components so this is not an issue. But for this gem's test setup, we need to require it explicitly.
require "job-iteration" | ||
require "active_job" |
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.
active_job
is already be required by job-iteration
.
require "job-iteration" | ||
require "job-iteration/test_helper" | ||
|
||
require "globalid" | ||
require "sidekiq" | ||
require "resque" | ||
require "active_job" |
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.
active_job
is already be required by job-iteration
.
@@ -13,6 +13,7 @@ jobs: | |||
ports: | |||
- 6379:6379 | |||
strategy: | |||
fail-fast: false |
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.
From my experience maintaining projects with a complicated dependency matrix, it's best not to fail fast on these kinds of builds so we can get a better sense on what's failing.
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.
Thank you for fixing this!
ActiveSupport::LoggerThreadSafeLevel
depends onlogger
, but doesn't require it explicitly prior to 7.1. In the real application, thelogger
is likely required by other components so this is not an issue. But for this gem's test setup, we need to require it explicitly.