Skip to content

add abstraction layer into redis client #5

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

Closed
wants to merge 5 commits into from
Closed

add abstraction layer into redis client #5

wants to merge 5 commits into from

Conversation

veelasky
Copy link
Contributor

Add abstraction layer into redis client, so the developers can bring their own client configuration.

@veelasky
Copy link
Contributor Author

@palicao actually all tests are passed, but somehow the code exited with code 1, is there anything to add in phpunit.xml config ?

@palicao
Copy link
Owner

palicao commented Dec 18, 2019

@palicao actually all tests are passed, but somehow the code exited with code 1, is there anything to add in phpunit.xml config ?

Yes, i noticed, and that's why I deleted my previous comment!
I think the problem is that scrutinizer decided to update PHP version to 7.3, and in this version apparently xdebug is not active, so no code coverage can be calculated.

I had a similar problem in another project, so I explicitly required php 7.2 to be installed. Can you please check this .scrutinizer file? https://github.com/palicao/phpRebloom/blob/master/.scrutinizer.yml and apply the needed changes? Basically it's about adding

environment:
  php:
    version: 7.2

in each node.

Or if you prefer I can try to fix it but you'll need to wait at least tomorrow.

@veelasky
Copy link
Contributor Author

I should add that this is PR is a breaking changes, you should consider move it to different branch, or release all new version all together.

@palicao
Copy link
Owner

palicao commented Dec 19, 2019

@veelasky I think the problem with scrutinizer is even bigger than I thought. I am very sorry for that. I will try to fix it ASAP and come back to this PR.

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