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

Add resque-scheduler 5 support #171

Merged

Conversation

and9000
Copy link
Contributor

@and9000 and9000 commented Sep 11, 2021

Hello,

resque-retry depends on resque-scheduler version ~> 4.0 but it's master branch (which contains useful fixes like this) already bumped version 5.

This PR enables using master's branch of resque-scheduler with resque-retry.

I've tested this PR locally adding gem 'resque-scheduler', github: 'resque/resque-scheduler' to the Gemfile and everything is green (I don't know how to include this test on the testsuite, please suggests me).

Thanks,
Andrea

@and9000 and9000 changed the title Add resque-scheduler master support Add resque-scheduler 5 support Sep 11, 2021
@jzaleski
Copy link
Collaborator

@and9000 from a test-coverage perspective, I think we're alright. However since you added ruby-3.0 support w/ #170, could you please re-base it in so we test against 3.0 here as well?

* resque-scheduler dependency >= 4.0 <6.0
@and9000 and9000 force-pushed the add_resque_scheduler_master_support branch from bc580c9 to b909e56 Compare September 12, 2021 16:00
@and9000
Copy link
Contributor Author

and9000 commented Sep 12, 2021

@and9000 from a test-coverage perspective, I think we're alright. However since you added ruby-3.0 support w/ #170, could you please re-base it in so we test against 3.0 here as well?

Sure, done!

Copy link
Collaborator

@jzaleski jzaleski left a comment

Choose a reason for hiding this comment

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

LGTM

@jzaleski jzaleski merged commit f53624e into lantins:master Sep 13, 2021
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.

2 participants