Predis 1.0 in branch 1.1

[theUniC]

Hi,

I've reproduced all the work to integrate Predis 1.0 into the 1.1 branch.

[bastnic]

👍

[michaelthieulin]

👍

[theUniC]

Do I miss something left to do?

[TNAJanssen]

+100

[ternel]

Nice 👍

[damienalexandre]

You also needs to change the suggest:

 "predis/predis": "If you want to use predis (currently only v0.8.x is supported).",

[theUniC]

Sorry for the delay. I've already fixed composer.json and the test description.

[TomAdam]

@snc would be great to get this merged.

[badlamer]

+1

[fdeweger]

+1

[piotrantosik]

Any update on this?

[mmoreram]

👍 Any update on that?

[mmoreram]

@snc Would be really nice, if you are no longer available for the maintenance of this project, to create a new organization, adding this project and allowing other people to do that essential work.

Please man, do that...

[dominics]

@mmoreram bravo

@snc or at least remove the 'project: maintained' badge so people know what they're getting themselves into. There are serious isses with open PRs over six months old (cross-connection writes, etc.).

[Seldaek]

I sent an email to snc because he might not get github notifications in his inbox.

[snc]

Oh damn, big sorry! Thanks for mailing me @Seldaek! I'll merge this now.

[mmoreram]

Thanks @Seldaek

[mmoreram]

@snc would be really nice to tag 1.1.9 in order to create stable dependencies between projects :)

[snc]

@mmoreram Just pushed the tag.

[mmoreram]

@snc Thanks :)

[javihgil]

Hi, I've just update my composer project (configured package is 1.1.*) and I get this error after updating to 1.1.9:

PHP Fatal error: Class 'Predis\Profile\Factory' not found in vendor/snc/redis-bundle/Snc/RedisBundle/DependencyInjection/SncRedisExtension.php on line 158

Maybe it's an error of the Predis package, but just for your info.

Thanks

[Seldaek]

You need to require predis ^1.0 to use 1.1.9.. which is kind of a BC break. I don't think this should have been released as is.. If the code worked with predis 1.0 or 0.8 it would be alright I guess, but that doesn't seem to be possible.

[javihgil]

Thanks, that solves my problem

[digitalkaoz]

im on @Seldaek's side..ugly avoidable BC break which could be simple solved by a class_exists? and that in a patch level -.-

[snc]

@digitalkaoz Are you sure a simple class_exists would solve this? I'm very busy right now and don't have the time to check this... I'm thinking about reverting this PR and instead merge it into the master branch.

[Seldaek]

class_exists isn't good enough IMO, as you are extending classes that got renamed.. But yes doing this in a patch version isn't very nice :) Doing it in master would be a start but the question is can we safely upgrade to master or is it breaking everything?

[digitalkaoz]

i dont know i guess this might work?

if (class_exists(\Predis\Profile\ServerProfile::class)) {
    $profileDef = new Definition(get_class(\Predis\Profile\ServerProfile::get($client['options']['profile'])));
else {
   $profileDef = new Definition(get_class(\Predis\Profile\Factory::get($client['options']['profile'])));
}

wouldnt that work? or even vice versa, some the both class didnt exists the past right?

[abraovic]

Hi @snc,

you might update doc https://github.com/snc/SncRedisBundle/blob/master/Resources/doc/index.md for predis version. It is still 0.8.x-dev which causes this problem.

Best,
Ante.

[liuggio]

In production i had to patch this bundle using the @digitalkaoz solution
Easy to reproduce the website is https://poser.pugx.org
agaist this repo https://github.com/liuggio/badge-poser/tree/flat-square
/cc @snc

-EDIT-
~~The problem persist on the dep.inj. definition of Parameters and Options. ~~
please revert or patch it.

-EDIT2-
my fault I messed up with the PHP server version ...