-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Support SMISMEMBER #2077
Support SMISMEMBER #2077
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spotted a few things cross-PR in reviewing these, notes inline! Thanks again, these PRs are fantastic and you're cranking 'em out <3
/// <returns> | ||
/// <see langword="true"/> if the element is a member of the set. | ||
/// <see langword="false"/> if the element is not a member of the set, or if key does not exist. | ||
/// </returns> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case doc should say an array of these true/false, matching position, e.g.
/// <returns> | |
/// <see langword="true"/> if the element is a member of the set. | |
/// <see langword="false"/> if the element is not a member of the set, or if key does not exist. | |
/// </returns> | |
/// <returns> | |
/// An array of booleans corresponding to <paramref name="values"/>, for each: | |
/// <see langword="true"/> if the element is a member of the set. | |
/// <see langword="false"/> if the element is not a member of the set, or if key does not exist. | |
/// </returns> |
What happens when the key isn't present, all falses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
Co-authored-by: Nick Craver <nrcraver@gmail.com>
Co-authored-by: Nick Craver <nrcraver@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good - thank you for this!
Pushed one last tidy on the tests, we should nuke the key when starting to ensure a consistent test run - after that clears will merge in. |
Right 😅 |
https://redis-stack.io/commands/smismember/
#2055