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

Changed redis "exists" method calls to "exists?" #306

Conversation

matt-may
Copy link

Redis "exists" method calls were changed to "exists?" to address a deprecation warning from the redis-rb gem.

Addresses #305.

Redis "exists" method calls were changed to "exists?" to address a deprecation warning from the redis-rb gem.
@coveralls
Copy link

Coverage Status

Coverage decreased (-19.03%) to 78.938% when pulling 4d9c1dd on matt-may:fix/handle-corner-case-deprecation-warning-from-redis-305 into 41990aa on moove-it:master.

@t27duck
Copy link
Contributor

t27duck commented Jul 24, 2020

It's possible the mock-redis gem needs to be bumped for the tests to pass.

Also, since the sidekiq update requires the most recent version of the redis gem which is driving this change, the minimum version of sidekiq may need to be bumped in the gemspec as well.

@iax7
Copy link

iax7 commented Jan 6, 2021

why this small change has delayed so much from merging?

@akhil-gautam
Copy link

@matt-may Are there any plans to get this merged?

@rmm5t
Copy link

rmm5t commented Feb 3, 2021

why this small change has delayed so much from merging?

Because the tests are failing.

It's possible the mock-redis gem needs to be bumped for the tests to pass.

Agreed.

Also, since the sidekiq update requires the most recent version of the redis gem which is driving this change, the minimum version of sidekiq may need to be bumped in the gemspec as well.

I think just the gemspec here needs to be updated.

  s.add_dependency 'redis', '~>= 4.2'

@rmm5t
Copy link

rmm5t commented Feb 3, 2021

See #329 which includes @matt-may's deprecation fix, but also fixes the specs as well while upgrading necessary dependencies.

@marcelolx
Copy link
Member

Thanks for the PR @matt-may! I'll be closing this since it has been fixed on #335

@marcelolx marcelolx closed this May 17, 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.

None yet

7 participants