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

fix(server): zunion now supports variadic arguments #717

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Conversation

romange
Copy link
Collaborator

@romange romange commented Jan 22, 2023

No description provided.

@romange romange requested a review from adiholden January 22, 2023 16:44
@romange romange force-pushed the VariadicZunion branch 3 times, most recently from 9ca9745 to 9ccad8b Compare January 22, 2023 17:18
EXPECT_THAT(resp.GetVec(), ElementsAre("c", "2", "d", "2"));

resp =
Run({"zunion", "z1", "z2", "z3", "weights", "1", "1", "2", "aggregate", "max", "withscores"});
resp = Run({"zunion", "3", "z1", "z2", "z3", "weights", "1", "1", "2", "aggregate", "max",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that we have only checks with 3 arguments, maybe change one of the cases to check different number

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM

1. Before that we did no support a real syntax with <numkey> argument,
now we do.

2. Fix warnings.

Signed-off-by: Roman Gershman <[email protected]>
@romange romange merged commit ac44a1f into main Jan 23, 2023
@romange romange deleted the VariadicZunion branch January 23, 2023 12:16
@romange romange mentioned this pull request Feb 20, 2023
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