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

Fix deprecated uses of Redis#pipelined #438

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

casperisfine
Copy link

Context: redis/redis-rb#1059

The following is deprecated

redis.pipelined do
  redis.get(key)
end

And should be rewritten as:

redis.pipelined do |pipeline|
  pipeline.get(key)
end

Functionally it makes no difference.
This API is available since Redis 3.0.

Context: redis/redis-rb#1059

The following is deprecated
```ruby
redis.pipelined do
  redis.get(key)
end
```

And should be rewritten as:
```ruby
redis.pipelined do |pipeline|
  pipeline.get(key)
end
```

Functionally it makes no difference.
This API is available since Redis 3.0.
files_set.map do |key|
@redis.hgetall(key)
@redis.pipelined { |pipeline|
files_set.each do |key|
Copy link
Owner

Choose a reason for hiding this comment

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

any reason the map moved to an each?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes because the return value doesn't matter. pipelined doesn't care about what the block returns.

Copy link
Owner

Choose a reason for hiding this comment

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

gotch, yep all seems good.

Copy link
Owner

@danmayer danmayer left a comment

Choose a reason for hiding this comment

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

merged thanks

@natematykiewicz
Copy link

@danmayer You said merged, but you only approved. Not sure if you realized that.

@danmayer danmayer merged commit 1306098 into danmayer:main Feb 9, 2022
@danmayer
Copy link
Owner

danmayer commented Feb 9, 2022

oops sorry merged... I will try to get a release out in the next day or two @natematykiewicz / @casperisfine

@casperisfine
Copy link
Author

No big deal It's just a warning and people have a flag to silence them.

@danmayer
Copy link
Owner

OK, sorry for the delay, this has been released in 5.2.2

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.

4 participants