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.

Sign up for GitHub

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

Jump to bottom

Node: convert VerbatimString into Uint8Array #1588

Merged
merged 1 commit into from
Jun 18, 2024
Merged

Node: convert VerbatimString into Uint8Array #1588

merged 1 commit into from
Jun 18, 2024

Conversation

eifrah-aws
Copy link
Contributor

Convert VerbatimString into binary safe string

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@eifrah-aws eifrah-aws added the node Node.js wrapper label Jun 17, 2024
@eifrah-aws eifrah-aws requested a review from a team as a code owner June 17, 2024 12:15
@eifrah-aws eifrah-aws changed the title Node: convert VerbatimString into Uint8Array Node: convert VerbatimString into Uint8Array Jun 17, 2024
acarbonetto
acarbonetto reviewed Jun 17, 2024
View reviewed changes
Copy link
Collaborator

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

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

I was under the impression that only BulkString required converting to binary data. Can't we keep SimpleString and VerbatimString in a String format?

@eifrah-aws
Copy link
Contributor Author

@acarbonetto not really. VerbatimString from the RESP3 spec:

Verbatim string: a binary safe string that should be displayed to humans without any escaping or filtering. For instance the output of LATENCY DOCTOR in Redis.

https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md

@eifrah-aws eifrah-aws merged commit 0369b9c into valkey-io:main Jun 18, 2024
8 checks passed
@eifrah-aws eifrah-aws deleted the node-verbatim-string-binary branch June 18, 2024 10:29
acarbonetto pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 18, 2024
@eifrah-aws @acarbonetto
Node: convert VerbatimString into Uint8Array (valkey-io#1588)
2c585ee 
Node: convert `VerbatimString` into Uint8Array
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 19, 2024
@eifrah-aws
Node: convert VerbatimString into Uint8Array (valkey-io#1588)
5a1c42b 
Node: convert `VerbatimString` into Uint8Array
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
@eifrah-aws @cyip10
Node: convert VerbatimString into Uint8Array (valkey-io#1588)
c2c04d9 
Node: convert `VerbatimString` into Uint8Array
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acarbonetto acarbonetto acarbonetto left review comments

@avifenesh avifenesh avifenesh approved these changes

Assignees
No one assigned
Labels
node Node.js wrapper
Projects
Valkey-GLIDE - internal
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eifrah-aws @avifenesh @acarbonetto