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

Specify Redis gems as required dependencies #154

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stanhu
Copy link
Collaborator

@stanhu stanhu commented May 12, 2023

We recently came across a Ruby 3 compatibility issue with redis-namespace and MailRoom that required updating to redis-namespace v1.8.x (https://github.com/resque/redis-namespace/blob/master/CHANGELOG.md). Since the gemspec did not specify a version, we have to install the required Redis gems by hand and missed the need for updating.

Adjust tests to support both Redis v4 and v5 gems. Redis v4 may still be needed for Rails 6 compatibility.

@stanhu
Copy link
Collaborator Author

stanhu commented May 12, 2023

It looks like the specs need to be updated for Redis 5: https://github.com/tpitale/mail_room/actions/runs/4961109521/jobs/8877471798

@stanhu stanhu force-pushed the sh-redis-namespace-required branch 3 times, most recently from 16d5f65 to 6a44dd4 Compare May 12, 2023 19:01
We recently came across a Ruby 3 compatibility issue with
redis-namespace and MailRoom that required updating to redis-namespace
v1.8.x
(https://github.com/resque/redis-namespace/blob/master/CHANGELOG.md).
Since the gemspec did not specify a version, we have to install
the required Redis gems by hand and missed the need for updating.

Adjust tests to support both Redis v4 and v5 gems. Redis v4
may still be needed for Rails 6 compatibility.
@stanhu stanhu force-pushed the sh-redis-namespace-required branch from 6a44dd4 to 31953a9 Compare May 12, 2023 19:04
@stanhu
Copy link
Collaborator Author

stanhu commented May 12, 2023

@tpitale Curious what you think about this pull request. In general, I think we should move move of the development gems into required gems so we can be sure that the code here works against the specific gem versions, but I can understand that C extensions might be a concern.

@stanhu
Copy link
Collaborator Author

stanhu commented Dec 8, 2023

@tpitale What do you think of this pull request and #161?

@tpitale
Copy link
Owner

tpitale commented Dec 8, 2023

I think I worried about this because not everyone uses this delivery method. But I'll have to look on a computer the next chance I get.

If you don't hear from me by Monday, don't hesitate to ping me again.

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