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

First preparations for release 0.4.0 #8

Merged
merged 8 commits into from
Jun 20, 2024
Merged

Conversation

sosthene-nitrokey
Copy link
Contributor

@sosthene-nitrokey sosthene-nitrokey commented Jun 19, 2024

  • Remove cbor feature
  • Remove Deref impl to heapless and replace it with a impl of Deref<Target = [u8]>
  • Remove try_from(f: impl FnOnce(&mut [u8]) -> Result<usize, ()> API
    AFAIK this is only used in trussed-utils to serialize cbor, which can be done through dedicated cbor-smol methods instead.
  • Remove insert_slice_at
    This is only used in fido-authenticator to add data to the beginning of the buffer. This can easily be made in 3 lines of code without using internal APIs.
  • Add From<Bytes<N>> for Vec<u8, N> and vice-versa
  • Make default features required (so that we can feature-flag the heapless parts of the public API in the future).
  • Renamed resize_default to resize_zero since we know that Default means Zero here.

@robin-nitrokey
Copy link
Member

Can you explain your plans for the default feature? AFAIS, your implementation only exposes heapless::Vec via the From implementations, so I think we could already put them behind a heapless-0.8 feature flag.

@sosthene-nitrokey
Copy link
Contributor Author

That way we don't have to do this now, if we want in the feature we can add a default-enabled heapless-0.8 feature flag and remove the requirement for the default feature flag.

But if before that we realize that AsMut is required, we can just do another release where we add it, and drop the idea of being able to bump heapless without a major version bump.

Copy link
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

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

I would introduce the heapless-0.8 feature directly because it would make it explicit which crates actually use the conversions. (I suspect it will only be one or two.) And I don’t think it makes a difference for a potential update to heapless 0.9. Best case we can just add a feature for it, or we need a breaking release anyway where we can also change the features. But it’s not a big issue so I’m also fine with the default approach.

@sosthene-nitrokey
Copy link
Contributor Author

I prefer the default approach so that it doesn't require us to document features we might never need to use.

@robin-nitrokey
Copy link
Member

Just one final observation from my side: If we use the default approach and the migrate from heapless 0.8 to 0.9, we need to update all crates using heapless-bytes to get rid of heapless 0.8. If we would directly use the feature, we would only need to update the crates that enabled the feature.

@sosthene-nitrokey
Copy link
Contributor Author

That's a good point.

I'll move to directly have the heapless-0.8 feature then.

@sosthene-nitrokey sosthene-nitrokey merged commit e4f19c1 into main Jun 20, 2024
1 check passed
@robin-nitrokey robin-nitrokey mentioned this pull request Aug 8, 2024
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