Skip to content

Conversation

@nigelis
Copy link
Contributor

@nigelis nigelis commented Apr 28, 2021

No description provided.

SInter(ctx context.Context, keys ...string) *StringSliceCmd
SInterStore(ctx context.Context, destination string, keys ...string) *IntCmd
SIsMember(ctx context.Context, key string, member interface{}) *BoolCmd
SMIsMember(ctx context.Context, key string, members ...interface{}) *BoolSliceCmd
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we should change SIsMember to accept multiple args and use smismember if len(members) > 1. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand that these two commands are quite similar, but changing SIsMember to accept multiple args breaks the API compatibility. The current type of return value is *BoolCmd, while the desired is *BoolSliceCmd.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we consider modifying it in v9? they function exactly the same.

Copy link
Contributor Author

@nigelis nigelis Apr 28, 2021

Choose a reason for hiding this comment

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

This looks acceptable to me.

I can modify the SIsMember and open another PR to branch v9 after you draw the conclusion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I haven't realized they return different results. No need to merge them then.

@vmihailenco vmihailenco merged commit b45fc83 into redis:master Apr 29, 2021
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.

3 participants