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

Make installing dalli and redis gem optional #18

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

doits
Copy link
Contributor

@doits doits commented Oct 15, 2021

fixes #3 and removes a deprecation warning for Dalli >= 3.0.0

Basically it blows up when doing Suo::Client::Memcached.new or Suo::Client::Redis.new without the respective gem and not passing in a custom client.

I added the check for the custom client in case somebody uses a special client without the official dalli or redis gem.

@doits doits force-pushed the optional_memcached_redis branch from 23f163d to 4a4fde6 Compare October 15, 2021 15:37
@doits doits force-pushed the optional_memcached_redis branch from 4a4fde6 to 0675462 Compare October 15, 2021 15:38
@doits doits force-pushed the optional_memcached_redis branch from 77b9693 to 21eb4d9 Compare October 15, 2021 16:00
@doits doits force-pushed the optional_memcached_redis branch from 21eb4d9 to 1fa3a39 Compare October 15, 2021 16:02
@doits
Copy link
Contributor Author

doits commented Oct 15, 2021

I think the PR is ready now. What do you think?

If you merge and release this, it should be a bigger version change (maybe even 1.0.0??).

@mieko
Copy link

mieko commented Nov 19, 2021

@doits Thanks for this branch. It's been working great for me.

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.

Require either redis or dalli but not both
2 participants