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

feat: support predis #72

Merged
merged 9 commits into from
Apr 8, 2024
Merged

feat: support predis #72

merged 9 commits into from
Apr 8, 2024

Conversation

godruoyi
Copy link
Owner

@godruoyi godruoyi commented Apr 6, 2024

  • fix tests
  • update readme

@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.74%. Comparing base (dec895d) to head (f3395ab).
Report is 1 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #72      +/-   ##
============================================
+ Coverage     98.71%   98.74%   +0.02%     
- Complexity       91       94       +3     
============================================
  Files             7        8       +1     
  Lines           234      239       +5     
============================================
+ Hits            231      236       +5     
  Misses            3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@godruoyi godruoyi mentioned this pull request Apr 6, 2024
composer.json Outdated Show resolved Hide resolved
@godruoyi godruoyi self-assigned this Apr 6, 2024
Copy link
Contributor

@Bilge Bilge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't help but notice this does not follow my suggestion to use the adapter pattern, but instead implements an entirely separate SequenceResolver. I still think the adapter pattern would work better, unless you have some specific reason why you disagree?

src/PHPRedisSequenceResolver.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
src/PHPRedisSequenceResolver.php Outdated Show resolved Hide resolved
src/RedisSequenceResolver.php Outdated Show resolved Hide resolved
@godruoyi godruoyi changed the title [wip] feat: support predis feat: support predis Apr 7, 2024
@godruoyi godruoyi added the Ready to release Ready to push to master and then publish a new version label Apr 7, 2024
@godruoyi godruoyi merged commit b8dfbc3 into master Apr 8, 2024
5 checks passed
@godruoyi godruoyi deleted the lianbo/support-predis branch April 8, 2024 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to release Ready to push to master and then publish a new version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants