Deprecation warning by using `redis.sadd` with Redis 4.8.0

[martingregoire]

Describe the bug

When using split with Redis 4.8.0 in our app, we see the following deprecation warning:

Redis#sadd will always return an Integer in Redis 5.0.0. Use Redis#sadd? instead.

Expected behavior

There should be no deprecation warning.

Additional context

Please see https://github.com/redis/redis-rb/blob/4.x/CHANGELOG.md#480

As far as I see in the code, redis.sadd is only called here: https://github.com/splitrb/split/blob/5092ef2972be65138ebe8171763c7632c8123983/lib/split/redis_interface.rb#L22-L24

The method add_to_set is only called from: https://github.com/splitrb/split/blob/5092ef2972be65138ebe8171763c7632c8123983/lib/split/experiment.rb#L464-L467

Proposal

As the returned value from redis.sadd is not used, a fix could be to follow the suggestion from the warning and change add_to_set to use redis.sadd? instead of redis.sadd if available:

    def add_to_set(set_name, value)
      return redis.sadd?(set_name, value) if redis.respond_to?(:sadd?)

      redis.sadd(set_name, value)
    end

[andrehjr]

I think that's fair. Do you want to open an PR?

sadd? is only available after redis 4.8.. and we're still on 4.2.

[martingregoire]

Yes, I'll try to take a look at the tests and then create a PR.

Updating the Redis dependency to 4.8 might be too much, right? That's why I thought about adding the "use sadd? if available" logic.

[andrehjr]

For now, checking if sadd? is available is good enough.

As 4.8 was released in August 22, bumping the dependency might be too soon. But it will happen at some point

[martingregoire]

Please have a look at the PR if your time permits. :wrench:

[andrehjr]

Closed by #701