Skip to content

Conversation

isabelatkinson
Copy link
Contributor

///
/// Padding must be within 0-7 inclusive. Padding must be 0 or unspecified if the provided
/// vector is empty.
pub fn new(vector: Vec<u8>, padding: impl Into<Option<u8>>) -> Result<Self> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the spec requires this type to be valid upon construction, hence the opaque type and result-returning new method

/// let binary = Binary::from(vector);
/// ```
///
/// `Vector` serializes to and deserializes from a `Binary`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's much value in treating this type as anything other than a convenience wrapper around Binary when it comes to serde, but we can revisit if a use case arises

@isabelatkinson isabelatkinson marked this pull request as ready for review January 23, 2025 20:42
Copy link
Contributor

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

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

LGTM!

@isabelatkinson isabelatkinson merged commit 098e1a1 into mongodb:main Jan 24, 2025
11 checks passed
@isabelatkinson isabelatkinson deleted the vector-subtype branch June 5, 2025 16:25
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