Skip to content
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 LCS #767

Merged
merged 4 commits into from
Feb 18, 2025
Merged

Add LCS #767

merged 4 commits into from
Feb 18, 2025

Conversation

arbha1erao
Copy link
Contributor

return cmd
}

func (cmd *LCSCmd) from(res rueidis.RedisResult) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parsing function seems incorrect. Is it compatible with https://github.com/redis/go-redis/blob/196fc9b21ac460f0d5251d349e4249e2ffedb9ff/command.go#L4510?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. I misunderstood what was required. I’ve added a new commit with the necessary changes.

@arbha1erao arbha1erao marked this pull request as draft February 17, 2025 14:17
@arbha1erao arbha1erao requested a review from rueian February 17, 2025 20:09
@arbha1erao arbha1erao marked this pull request as ready for review February 17, 2025 20:09
switch cmd.readType {
case 1:
// match string
if lcs.MatchString, err = res.ToString(); err == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if lcs.MatchString, err = res.ToString(); err == nil {
if lcs.MatchString, err = res.ToString(); err != nil {

Shouldn't this be err != nil?

}
case 2:
// match len
if lcs.Len, err = res.AsInt64(); err == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if lcs.Len, err = res.AsInt64(); err == nil {
if lcs.Len, err = res.AsInt64(); err != nil {

Shouldn't this be err != nil?

}
case 3:
// read LCSMatch
if msgMap, err := res.AsMap(); err == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if err != nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

@arbha1erao arbha1erao requested a review from rueian February 17, 2025 20:49
@rueian rueian merged commit c571913 into redis:main Feb 18, 2025
27 checks passed
@rueian
Copy link
Collaborator

rueian commented Feb 18, 2025

Thank you @arbha1erao

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants