-
Notifications
You must be signed in to change notification settings - Fork 339
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
Fixes #189 #190
Fixes #189 #190
Conversation
I'll double check that and the failing CI too. |
This is actually a combo, because it fixes the bug and implicitly adds the possibility to set global options which apply to all Redis::Store instances when multiple hosts are set.
@jodosha you were right of course defaults are not required, I removed them and squashed my commits. |
This will fix #37 as well |
An attempt to fix this issue was made here too: #169 but that implementation doesn't to take into account the connection string format, not the hash format. |
Another attempt to fix this issue was made here #172 with hash key based approach I personally dislike. |
@freegenie After I merged this, master build is broken, would you please take care of this? |
@jodosha sure |
Before this change, this app was running with a patched version of the redis-activesupport gem which fixed a pretty nasty bug ( redis-store/redis-store#166). This bug was fixed upstream (redis-store/redis-store#190) and released with redis-activesupport 3.2.4 so we don't need to maintain a separate branch any more.
This is actually a combo, because it fixes the bug and implicitly
adds the possibility to set global options which apply to all
Redis::Store instances when multiple hosts are set.