#207 Feature: get in set #1238
#207 Feature: get in set #1238
Conversation
|
There seems to be a small test failure. Probably unrelated to your changes. Should be fixed with a rebase hopefully (or you can do it manually) |
apoorvyadav1111
force-pushed
the
feature/201-get-in-set
branch
from
ff99949 to
853309b
Compare
|
Thanks @JyotinderSingh, I just rebased + fixed the issue, and its passing unit tests now. |
apoorvyadav1111
force-pushed
the
feature/201-get-in-set
branch
from
4f7aa41 to
b13bb83
Compare
internal/eval/store_eval.go
Outdated
| } | ||
| getOldVal = oldVal.Result |
There was a problem hiding this comment.
Can you please share this as a code suggestion? that way I would be able to accurately test it in local.
|
Hi @tejasa97 , thanks for your review , I ll be addressing them soon. |
There was a problem hiding this comment.
Thanks for adding this feature @apoorvyadav1111! The changes mostly look good, we can proceed with merge once the review comments by @tejasa97 are addressed.
|
Hi @JyotinderSingh , @tejasa97, have addressed the comments, please review. |
apoorvyadav1111
force-pushed
the
feature/201-get-in-set
branch
from
2212f27 to
bbdca95
Compare
apoorvyadav1111
force-pushed
the
feature/201-get-in-set
branch
from
bbdca95 to
aecede9
Compare
| func generateFingrprint(cmdName string, key string) string { | ||
| cmd := cmd.DiceDBCmd{ | ||
| Cmd: cmdName, | ||
| Args: []string{key}, | ||
| } | ||
| return fmt.Sprintf("%d", cmd.GetFingerprint()) | ||
| } | ||
|
|
There was a problem hiding this comment.
fingerprint generation logic can be hidden away from the test, we can simply assert the behavior using a static string.
There was a problem hiding this comment.
Understood. have done this change in the latest commit.
Hi team,
Fixes: #207
Added support for GET option in SET.