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

Deprecate == and != from Redis::Future #876

Merged
merged 2 commits into from Sep 19, 2019

Conversation

@GustavoCaso
Copy link
Contributor

GustavoCaso commented Sep 19, 2019

For more information please check #875

Most developers when using == after calling a redis command they expect to compare the result returned from Redis.

Inside a pipelined, we are operating with Redis::Future instances that are BasicObject

BasicObject#== is doing identity comparison instead of value comparison which is less intuitive for developers.

Example:

result = nil

redis.pipelined do 
  result = redis.set('foo', 'bar') == "OK"
end

puts result 
# => false

By removing the ability to use == with a redis future developers would get NoMethodError and this helps to have more consistent errors when operating with Redis::Future inside a pipelined block

This helps to have a more consistent errors when operating with
Redis::Future inside a pipelined block
@GustavoCaso GustavoCaso force-pushed the GustavoCaso:undef-==-from-redis-future branch from abf097b to d3526cc Sep 19, 2019
@casperisfine
Copy link

casperisfine commented Sep 19, 2019

A couple things:

  • I'm generally favorable to this, however redis being very widely used, I think it needs a deprecation period
  • There is also the != method
  • People can get tricked if they use Yoda comparisons "OK" == redis.set('foo', 'bar'), but I guess that's fine.
@GustavoCaso GustavoCaso changed the title Remove method == from Redis::Future Deprecate == and != from Redis::Future Sep 19, 2019
@GustavoCaso
Copy link
Contributor Author

GustavoCaso commented Sep 19, 2019

Example of trying to call the == or != method

bin/console
irb(main):001:0> redis = Redis.new
=> #<Redis client v4.1.3 for redis://127.0.0.1:6379/0>
irb(main):002:0> redis.pipelined do
irb(main):003:1* redis.set("foo", "bar") == 'OK"
irb(main):004:1' end
irb(main):005:1' ^C
irb(main):005:0> redis.pipelined do
irb(main):006:1* redis.set("foo", "bar") == 'OK'
irb(main):007:1> end

The method == and != is deprecated for Redis::Future and will be removed in 4.2.0 - You probably meant to call .value == or .value != (in /Users/gustavocaso/src/github.com/redis-rb/lib/redis/pipeline.rb:145:in `==')
=> ["OK"]
irb(main):008:0> redis.pipelined do
irb(main):009:1* redis.set("foo", "bar") != 'OK'
irb(main):010:1> end

The method == and != is deprecated for Redis::Future and will be removed in 4.2.0 - You probably meant to call .value == or .value != (in /Users/gustavocaso/src/github.com/redis-rb/lib/redis/pipeline.rb:145:in `==')
=> ["OK"]
irb(main):011:0>
@GustavoCaso
Copy link
Contributor Author

GustavoCaso commented Sep 19, 2019

I didn't have to deprecate != explicitly since != uses == underneath and we will see two deprecated messages.

@byroot byroot merged commit 6b894f8 into redis:master Sep 19, 2019
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@GustavoCaso GustavoCaso deleted the GustavoCaso:undef-==-from-redis-future branch Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.