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

feat: Add a Method to Support Dynamic Element Sizes in Sequence #44

Merged

Conversation

tareknaser
Copy link
Contributor

Rationale for this change

So far, all the methods to create a Sequence seem to be on statically sized types.
For more context, check out spaceandtimelabs/sxt-proof-of-sql#240 (comment)

What changes are included in this PR?

Implement from_raw_parts_with_size() for handling variable-sized elements.

Are these changes tested?

Yes. Added a test we_can_convert_a_slice_of_fixed_size_binary_to_a_sequence_with_correct_data() to verify the changes

@JayWhite2357
Copy link
Contributor

Sorry for the delay on this. @jacobtrombetta would be good to get your input here.

@jacobtrombetta
Copy link
Contributor

We should update the from_raw_parts method to use from_raw_parts_with_size.

pub fn from_raw_parts<T>(slice: &'a [T], is_signed: bool) -> Self {
   let element_size = core::mem::size_of::<T>();
   Self::from_raw_parts_with_size(slice, element_size, is_signed)
} 

This will allow us to minimize duplicate code.

@tareknaser
Copy link
Contributor Author

Thanks for the review. I’ve updated the PR to address your comments and rebased it on top of main. Let me know if there’s anything else

Copy link
Contributor

@jacobtrombetta jacobtrombetta left a comment

Choose a reason for hiding this comment

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

LGTM

@jacobtrombetta jacobtrombetta merged commit a2bb3c4 into spaceandtimelabs:main Oct 28, 2024
7 checks passed
@SxT-Release
Copy link

🎉 This PR is included in version 3.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@tareknaser tareknaser deleted the variable_sized_elements branch October 28, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants