-
Notifications
You must be signed in to change notification settings - Fork 166
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
Add FT.ALTER in OM JSON and Hash repo #645
Conversation
Signed-off-by: Vatsal <[email protected]>
Signed-off-by: Vatsal <[email protected]>
Hi @imvtsl, are you still working on testing? |
Hey @rueian Before I dive back in, I wanted to clarify—are we still good with the solution developed in this PR? |
The discussion doesn’t affect this PR. FT.ALTER itself is still a useful feature. |
Thank you for clarification. I will resume right away! |
Signed-off-by: Vatsal <[email protected]>
Signed-off-by: Vatsal <[email protected]>
Signed-off-by: Vatsal <[email protected]>
Hi @rueian Is there any style guide that we reference? If yes, can you share that so I can look it up make any required changes before I update this PR from Draft to Ready for Review. |
The tests are already looked good to me.
I think that was a bug caused by the wrong command definitions. The rueidis/hack/cmds/commands_search.json Lines 357 to 360 in 25821d0
I think we can have another PR to fix that. |
Thanks, I marked it as ready for review. I don't see the option to request review. @rueian can you review this PR?
Can I create the issue and raise PR if it's that small a change? |
Sure, thanks! |
I created this issue for it. I will raise PR for it as soon as this PR is merged. |
Signed-off-by: Vatsal [email protected]
Closes #632.