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 support for SafeByte and SafeBytes #30

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

arjunmahishi
Copy link
Contributor

@arjunmahishi arjunmahishi commented Jan 28, 2025

Currently, there is no direct way to write a safe byte or a byte slice to a SafeWriter. They have to be type-casted to either a rune (for byte) or a string (for []byte). It would be more efficient to add the types (and helpers) SafeByte and SafeBytes.

Also, UnsafeByte and UnsafeBytes already exist. So, introducing SafeByte and SafeBytes would also make the API more consistent.


This change is Reviewable

Currently, there is no direct way to write a safe byte or a byte slice
to a SafeWriter. They have to be type-casted to either a rune (for byte)
or a string (for []byte). It would be more efficient to add
the types (and helpers) SafeByte and SafeBytes.

Also, UnsafeByte and UnsafeBytes already exist. So, introducing SafeByte and
SafeBytes would also make the API more consistent.
@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Jan 28, 2025

CLA assistant check
All committers have signed the CLA.

@dhartunian dhartunian self-requested a review January 28, 2025 19:17
Copy link

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

LGTM

You probably want to add a tag with the next release version so we can bump in CRDB. Given this is a backwards compatible change, I think v1.1.6 is fine.

@arjunmahishi
Copy link
Contributor Author

arjunmahishi commented Jan 30, 2025

@dhartunian TFTR! Re: backward compatibility, two new methods are added to type SafeWriter interface. These methods are implemented in all types (within this package) that were conforming to this interface. But this is an exported interface. Would this still be considered backward compatible?


Edit: I think v1.1.6 should be fine since there isn't really a usecase for an external implementation of this interface. Plus, Go modules should ensure that there is no accidental upgrade.

@arjunmahishi arjunmahishi merged commit 2d146e2 into master Jan 30, 2025
6 checks passed
@arjunmahishi arjunmahishi deleted the support-for-safe-bytes branch January 30, 2025 06:08
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.

3 participants