search

resque / redis-namespace

This gem adds a Redis::Namespace class which can be used to namespace Redis keys.
http://redis.io
MIT License
695 stars 192 forks source link

Redis matrix test, dependabot, codeql, and redis upgrade #205

Closed iloveitaly closed 2 years ago

iloveitaly commented 2 years ago

It looks like redis 3.x is not supported by this gem and we should bump the redis requirement.

Here's an example run where 3.x is failing:

https://github.com/resque/redis-namespace/runs/7714438348?check_suite_focus=true

iloveitaly commented 2 years ago

cc @patricktulskie

PatrickTulskie commented 2 years ago

@iloveitaly Interesting. I have one project on redis-namespace 1.7.0 and redis 3.3.5 but it looks like everything else I have is on Redis 4+

Looks like the change in 1.8.0 Support variadic exists and exists? is the thing that is causing the test failure.

https://github.com/resque/redis-namespace/commit/cc09dc3477d924bf5bfb447275b5a8c01c22c9e3#diff-0da9d51fcbbae58729053f360d5f167a284c043cbefbf2a9f6626a510658449eR153

So yeah basically our choices are version bump or skip that test and/or add some backwards compatibility logic for Redis < 4. Feels like a version bump makes the most sense given how old 3 is and trying to maintain support for it is going to be a struggle.

byroot commented 2 years ago

@iloveitaly I don't consider myself a maintainer of this gem.

iloveitaly commented 2 years ago

@byroot got it! Looks like you had the most recent commits so wanted to keep you in the loop.

@PatrickTulskie Let's do a version bump. 3.x is very old at this point.

PatrickTulskie commented 2 years ago

@iloveitaly yep makes sense to me. Nice that we have a test matrix for this now. Should make future bumps a bit easier too.