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

[5.4] Convert zRangeByScore syntax for PhpRedis #17912

Merged
merged 1 commit into from
Feb 13, 2017

Conversation

tillkruss
Copy link
Contributor

This converts Predis':

['WITHSCORES' => true, 'LIMIT' => [$offset, $count]

To PhpRedis:

['withscores' => true, 'limit' => [$offset, $count]

@nrk
Copy link

nrk commented Feb 13, 2017

Just a related note: Predis doesn't really care about which casing style is used for command modifiers keywords as it converts them to uppercase internally, so you can pass these as WITHSCORES or withscores or even WiThScORES and it won't make any difference. The use of uppercase-by-default in Predis, in docs or code, is just for consistency with the Redis documentation.

@tillkruss
Copy link
Contributor Author

@nrk: Yeah, we use Predis as the standard and just make sure the syntax is converted when PhpRedis is used. What would really help us is a full documentation of all Predis methods and their supported signatures. I noticed that some methods can be called in 2-3 different ways.

@taylorotwell
Copy link
Member

Do we actually need this if the case doesn't matter?

@tillkruss
Copy link
Contributor Author

Yes, Predis doesn't care, PhpRedis does 😐

@taylorotwell taylorotwell merged commit 2095064 into laravel:5.4 Feb 13, 2017
@nrk
Copy link

nrk commented Feb 15, 2017

@tillkruss having proper documentation for Predis would be awesome but unfortunately I don't have enough free time for it and nobody seems motivated enough to submit contributions in this area. I know the test suite shouldn't really be used as a reference, but test units for each command cover all of their supported signatures.

@tillkruss
Copy link
Contributor Author

@nrk: Okay, thanks, I'll have a look!

@tillkruss tillkruss deleted the zrange-withscores branch April 16, 2017 18:06
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.

3 participants