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.6] Fix *scan methods for phpredis #24313

Merged

Conversation

ragingdave
Copy link
Contributor

@ragingdave ragingdave commented May 25, 2018

This adds the adapter methods to properly utilize PhpRedis commands for scan, sscan, hscan, and zscan. fixes #24222 .... take 3.

I think there was some confusion caused by myself perhaps, I fell into an old work habit where if a PR was rejected, that meant a new branch because enough was wrong that it was beyond fixing. Clearly that is not what was expected, so I noticed the merge that caused the failed tests was drawing from the original branch. I won't be making that mistake again and will just continue on the same branch for a feature, but just to be clear the branch name changed and I added a v2 on the end. I apologize for that....still relatively new to contributing.

To be clear the most current branch is fix-scan-methods-for-phpredis-v2. I do still have the old branch locally, so if it makes it easier I can just move the changes from this branch to the original and retarget the PR.

@ragingdave ragingdave changed the title Fix scan methods for phpredis v2 [5.6] Fix *scan methods for phpredis May 25, 2018
@taylorotwell taylorotwell merged commit 779aa7d into laravel:5.6 Jun 1, 2018
@taylorotwell
Copy link
Member

taylorotwell commented Jun 1, 2018

Reverted all this. Causes tests on master to break completely and just creates way too many merge conflicts. I'm never merging this on 5.6. You can try it submit it to master if you want or let someone else handle it.

/cc @tillkruss

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.

2 participants