Skip to content

Conversation

@sosthene-nitrokey
Copy link
Contributor

Efforts have been made to avoid breaking changes:

  • Since capacity and len were const, they have been kept on Deque, and the generic version on DequeInner are prefixed by storage and are not const
  • Iter<'a, T, const N: usize> is kept, and a new IterView type is added with conversions to and from both iter types. (I also simplified significantly the implementation of the Iterator, by using the standard library's iterator combinators).

By accepting a backward incompatible change, these APIs could be made more intuitive.

@Dirbaio
Copy link
Member

Dirbaio commented Jul 1, 2024

Iter, IterMut weren't public before becuase mod deque wasn't, so I think changing them is not breaking, I think? I'd just do the changes to them directly, keeps the code simpler.

The const len/capacity thing is unfortunate. I guess we can live with it for 0.8.x then clean it up for 0.9 which should be soon anyway (for example i'd like to make all per-container mods public and remove all the reexports at the crate root. the docs are getting hard to read with everything mixed up).

@sosthene-nitrokey
Copy link
Contributor Author

Oh, that's right for the iterators!

I would go even further and return directly iter::Chain<slice::Iter, _> in the methods. That would reduce the code we need to implement, and give us for free all the "goodies" that iterators can have (double ended, size_hint, trustedsize, skip...). Do you think it's a good idea?

@sosthene-nitrokey
Copy link
Contributor Author

sosthene-nitrokey commented Jul 1, 2024

Should I create a issue to track breaking changes for the future 0.9 release?

@Dirbaio
Copy link
Member

Dirbaio commented Jul 1, 2024

I would go even further and return directly iter::Chain<slice::Iter, _> in the methods.

perhaps that's a bit too far? we'd be leaking the details on how it's implemented, so we can't change it later if we find it doesn't work for some reason (can't think of why that'd be, but you never know...).

an alternative is RPIT but then the user can't name the iterator. this'd be the perfect use case for TAIT when it's stable... for now I think a wrapper struct is best even if it's a bit verbose.

Should I create a issue to track breaking changes for the future 0.9 release?

why not, that'd be great!

@Dirbaio Dirbaio added this pull request to the merge queue Jul 1, 2024
Merged via the queue into rust-embedded:main with commit 02cc494 Jul 1, 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