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

Add LMPOP #311

Merged
merged 10 commits into from
Nov 27, 2024
Merged

Add LMPOP #311

merged 10 commits into from
Nov 27, 2024

Conversation

Palladinium
Copy link
Contributor

Currently failing due to needing redis-rb v5 (see also #281 and #293)

@sds
Copy link
Owner

sds commented Nov 14, 2024

If you are interested in carrying this further, I would welcome an upgrade of redis-rb to v5. In any case, really appreciate you putting up a draft PR.

@Palladinium
Copy link
Contributor Author

I'll wait a week or two to see if #293 progresses, if not I'll take a stab at updating to v5 from scratch myself

@sds
Copy link
Owner

sds commented Nov 27, 2024

Shout out to @hieuk09 who submitted support for redis-rb v5 in #314.

You should be able to pick this up again after a rebase.

@Palladinium Palladinium marked this pull request as ready for review November 27, 2024 00:52
@Palladinium
Copy link
Contributor Author

A bunch of tests fail on my local environment, and also a bunch of CI checks fail too. They all seem unrelated to the LMPOP changes, so I'm not sure what's up. Do you have any ideas?

@sds
Copy link
Owner

sds commented Nov 27, 2024

@Palladinium the tests I see failing are all on Redis 6.2 and mention lmpop, so they seem relevant?

Lastly, the Overcommit failure seems like it would be solved by adding base64 to the Gemfile.

@Palladinium
Copy link
Contributor Author

Ah of course, LMPOP was only introduced in Redis v7, so those failures make sense

@sds
Copy link
Owner

sds commented Nov 27, 2024

Sorry, I forgot lmpop is new as of Redis 7. Can you update the tests to be skipped for Redis 6.2?

@sds sds enabled auto-merge (squash) November 27, 2024 01:08
@sds sds merged commit 7a2677f into sds:main Nov 27, 2024
10 checks passed
@sds
Copy link
Owner

sds commented Nov 27, 2024

Thank you for the quick turnaround! Released in 0.48.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants