-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
#650: Implementation HMGET
command
#743
Conversation
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.
Hi, the changes look good to me. Would it be okay for you to add a 3-4 line comment over the evalHMGET function describing general logic?
Thanks
Edit: Please confirm if #705 is a duplicate for this. If yes, would it be okay to close the PR stating a small comment? This will ensure it is not reviewed.
Hi @apoorvyadav1111, I've added the comments. Let me know if those are fine. |
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.
Logic looks good to me. I have left one minor comment. Please check
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 for these changes @vishnuchandrashekar. I have left a few comments.
@vishnuchandrashekar The PR is almost ready, let's try to address the comments and get it submitted soon. |
Hey @JyotinderSingh , I've updated the tests and the addressed the comments. Please have a look. |
Closes #650