Go: Implementing DEL command #2294
Conversation
MikeMwita
commented
Sep 15, 2024
MikeMwita
commented
- Implemented DEL command
| suite.runWithDefaultClients(func(client api.BaseClient) { | ||
| key1 := "testKey1" | ||
| key2 := "testKey2" | ||
| key3 := "testKey3" |
There was a problem hiding this comment.
Consider using keys specific to tests or uuid generator for unique keys as these keys might clash with other tests and lead to unpredictable results.
| // result, err := client.Del("key1", "key2", "key3") | ||
| // | ||
| // [valkey.io]: https://valkey.io/commands/del/ | ||
| Del(keys ...string) (int64, error) |
There was a problem hiding this comment.
Let's try to maintain the uniformity in the function signature. Either we can use keys []string here or pass keys ... string at other places.
There was a problem hiding this comment.
We don't use varargs in other clients, please, use []
go/api/commands.go
Outdated
| // Returns the number of keys that were removed. | ||
| // | ||
| // For example: | ||
| // result, err := client.Del("key1", "key2", "key3") |
There was a problem hiding this comment.
Please add how the result will look like. (You can refer the templates of other commands)
go/api/base_client.go
Outdated
| func (client *baseClient) Del(keys ...string) (int64, error) { | ||
| if client.coreClient == nil { | ||
| return 0, &ClosingError{"Del command failed. The client is closed."} | ||
| } |
There was a problem hiding this comment.
This is a redundant check and can be removed from here as it's already being checked in ExecuteCommand function.
| // result, err := client.Del("key1", "key2", "key3") | ||
| // | ||
| // [valkey.io]: https://valkey.io/commands/del/ | ||
| Del(keys ...string) (int64, error) |
There was a problem hiding this comment.
On referring to other clients like Java, Del command is not part of StringCommands interface and it's a part of GenericBaseCommands. Can we please add another interface and define it there?
| suite.runWithDefaultClients(func(client api.BaseClient) { | ||
| key1 := "testKey1" | ||
| key2 := "testKey2" | ||
| key3 := "testKey3" |
go/api/commands.go
Outdated
| @@ -371,4 +371,18 @@ type StringCommands interface { | |||
| // | |||
| //[valkey.io]: https://valkey.io/commands/getdel/ | |||
| GetDel(key string) (string, error) | |||
|
|
|||
| // Del removes the specified keys. A key is ignored if it does not exist. | |||
There was a problem hiding this comment.
Please copy docs from other clients to ensure that they are all aligned.
Add a cluster notice:
| // result, err := client.Del("key1", "key2", "key3") | ||
| // | ||
| // [valkey.io]: https://valkey.io/commands/del/ | ||
| Del(keys ...string) (int64, error) |
There was a problem hiding this comment.
We don't use varargs in other clients, please, use []
MikeMwita
force-pushed
the
implement-del-command
branch
3 times, most recently
from
db090e0
to
dfa827b
Compare
|
Can you please re-base this PR branch and re-submit? |
Signed-off-by: MikeMwita <[email protected]>
MikeMwita
force-pushed
the
implement-del-command
branch
from
dfa827b
to
a63d467
Compare
