Updated Predis to work with 1.*, not 0.8 #150
Conversation
| use Snc\RedisBundle\Logger\RedisLogger; | ||
|
|
||
| /** | ||
| * ConnectionWrapper | ||
| */ | ||
| class ConnectionWrapper implements SingleConnectionInterface | ||
| class ConnectionWrapper implements ConnectionInterface |
There was a problem hiding this comment.
This class should implement Predis\Connection\NodeConnectionInterface.
|
Just a side note: while Predis v1.0 is obviously better than the good old v0.8 under many aspects (at least for what concerns its internals), the latter is plenty stable and will be maintained for a while more for eventual bugfixes awaiting for a stable release of Redis 3.0. So while upgrading to Predis v1.0 is obviously recommended, there's no need to rush for existing codebases. |
|
Thanks @nrk -- I appreciate the look and advice. |
|
Thanks @mattmontgomery and @nrk. I think this would be a good start in conjunction with #126 for a 2.0 version. |
| @@ -129,7 +129,7 @@ private function addClientsSection(ArrayNodeDefinition $rootNode) | |||
| ->scalarNode('profile')->defaultValue('2.4') | |||
| ->beforeNormalization() | |||
| ->ifTrue(function($v) { return false === is_string($v); }) | |||
| ->then(function($v) { return sprintf('%.1F', $v); }) | |||
| ->then(function($v) { return sprintf('%.1f', $v); }) | |||
|
Some changes (session) you made have nothing to do with the predis update. Could you remove them please and send them in another PR if needed? |
|
+1 for supporting predis 1.x |
|
Any update on this? |
|
👍 |
|
Just to confirm, we tried this on a project with predis 1 and with a very quick test it seemed to work. So a tentative 👍 😄 |
|
@mattmontgomery have you seen my comments some months ago? :-) |
|
Given that @mattmontgomery has been out for months ... What If I fork his repository, remove the session changes and send another PR? |
|
This would be very nice! |
|
Ah, I really should have better notifications set up. Glad you've picked that up, @theUniC. |
|
Any news here? |
|
👍 |
1 similar comment
|
👍 |
|
Hi is there a reason this has not been merged in yet? I just tried to update to predis 1.0.1 and got Which this pull request appear to fix |
|
There is a second PR, #172, to continue with the work from @mattmontgomery. Unfortunately, this project appears to be discontinued since the owner has not said anything and there are no commits / merges since September from last year :( |
|
Oh I hadn't noticed that. That's a great shame if this is dead :( |
|
I haven't noticed this, too, sorry, #172 got merged. |
I'm not sure if you'll have any use for this or be interested in any of the changes, but these are changes I made to get SncRedisBundle working with Predis 1.0 -- take them or leave them. Tests look to be passing (with two failures that were present before the changes I've made.)