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

Implement the RESP3 null for the nil string and array #2017

Merged
merged 5 commits into from
Jan 15, 2024

Conversation

git-hulk
Copy link
Member

@git-hulk git-hulk commented Jan 14, 2024

For the null type in RESP2, the nil string is represented by $-1\r\n
and nil array is *-1\r\n. But after RESP3, they will be reduced into
a unified string _\r\n. So in this PR, most of them are renamed redis::MultiBulkString/Redis::NilString to connection::MultiBulkString and connection::NilString.

This closes #1993

@git-hulk
Copy link
Member Author

git-hulk commented Jan 14, 2024

This PR involves many files, but most of them are renaming the function.

@git-hulk git-hulk marked this pull request as ready for review January 15, 2024 01:52
@git-hulk git-hulk requested review from PragmaTwice, torwig, enjoy-binbin and mapleFU and removed request for PragmaTwice January 15, 2024 01:53
torwig
torwig previously approved these changes Jan 15, 2024
Copy link
Contributor

@torwig torwig left a comment

Choose a reason for hiding this comment

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

LGTM. Good job, @git-hulk !

For the null type in RESP2, the nil string is represented by `$-1\r\n`
and nil array is `*-1\r\n`. But after RESP3, they will be reduced into
an unify string `_\r\n`. So in this PR, most of them are renamed
redis::MultiBulkString/Redis::NilString to connection::MultiBulkString
and connection::NilString.
@git-hulk
Copy link
Member Author

@torwig I changed NilString in the last commit for the new command ZRANDMEMBER which was introduced in #2016, please retake a look.

torwig
torwig previously approved these changes Jan 15, 2024
Copy link

sonarcloud bot commented Jan 15, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
1.7% Duplication on New Code

See analysis details on SonarCloud

@git-hulk git-hulk merged commit d1acbcb into apache:unstable Jan 15, 2024
29 checks passed
@git-hulk
Copy link
Member Author

Thanks for @torwig @PragmaTwice review.

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.

Add RESP3 null type
3 participants