-
Notifications
You must be signed in to change notification settings - Fork 77
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
Go: Implement Bitop command #3384
Go: Implement Bitop command #3384
Conversation
Signed-off-by: Niharika Bhavaraju <[email protected]>
Signed-off-by: Niharika Bhavaraju <[email protected]>
Signed-off-by: Niharika Bhavaraju <[email protected]>
Signed-off-by: Niharika Bhavaraju <[email protected]>
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.
Please add changelog and fix PR description (needs a link to an issue)
Signed-off-by: Niharika Bhavaraju <[email protected]>
Signed-off-by: Niharika Bhavaraju <[email protected]>
Signed-off-by: Niharika Bhavaraju <[email protected]>
…ey-glide into niharika-bitopcmd
Signed-off-by: Niharika Bhavaraju <[email protected]>
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.
LGTM assuming last comments addressed
… niharika-bitopcmd
Signed-off-by: Niharika Bhavaraju <[email protected]>
Signed-off-by: Niharika Bhavaraju <[email protected]>
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.
Thanks!
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.
bitopkey2 := "{bitop_test}key2" | ||
destKey := "{bitop_test}dest" | ||
|
||
// Set initial values |
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.
Nice use of comments to help clarify examples.
go/integTest/shared_commands_test.go
Outdated
key2 := "{bitop_test}" + uuid.New().String() | ||
|
||
_, err := client.Set(key1, "foo") | ||
assert.Nil(suite.T(), err) |
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.
Would be better to use assert.NoError() in tests rather than assert.Nil(). The NoError() check is still a Nil check under the covers, but is more descriptive from a code perspective.
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.
done
Signed-off-by: Niharika Bhavaraju <[email protected]>
Go: Implement Bitop command
Issue link
This Pull Request is linked to issue : #3397