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

Change Z member type to string #2818

Merged
merged 3 commits into from
Dec 17, 2023
Merged

Change Z member type to string #2818

merged 3 commits into from
Dec 17, 2023

Conversation

ofekshenawa
Copy link
Collaborator

Closes #2817

@ofekshenawa ofekshenawa requested review from chayim and a team November 30, 2023 14:17
@ofekshenawa ofekshenawa merged commit e535124 into master Dec 17, 2023
9 checks passed
@jarno-rootz
Copy link

You don't do semantic versioning in go-redis? This is a breaking change.

@AlexanderYastrebov
Copy link
Contributor

This change breaks our code zalando/skipper#2806

# github.com/zalando/skipper/net
net/redisclient.go:404:47: cannot use val (variable of type int64) as string value in struct literal

as we used int64

AlexanderYastrebov added a commit to zalando/skipper that referenced this pull request Jan 2, 2024
There was a breaking change to go-redis library redis/go-redis#2818

I've checked how it handles interface{} command arguments and it
converts int64 to string so this change should fix the problem.

Closes #2806

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@knadh
Copy link
Contributor

knadh commented Jan 2, 2024

I would expect a breaking change only happens at major version updates.

This change seems to be a mistake. Going by the spec, all values in Redis are "strings", including Set(), which currently takes an interface{}. Changing the type in ZMember to string doesn't make any sense.

Also, this is a semver breaking change pushed in a minor version that just broke our codebase.

@ofekshenawa ofekshenawa deleted the os-fix-zset-member-type branch June 19, 2024 22:04
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.

zset member returned as an interface type instead of a string and is not type-safe
5 participants