-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
tokio-util: allow encoding borrowed buffers at LengthDelimitedCodec
#6533
Conversation
LengthDelimitedCodec
assert_ready_ok!(<MockFramedWrite as futures::Sink<Bytes>>::poll_ready( | ||
io.as_mut(), | ||
cx | ||
)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in the original issue, this change could break the existing code like this, so I'm not sure we can make this change right now.
Closing as it is a breaking change. |
I think it would make sense to documenting that this is a permanent state and won't be fixed. The rationale for documenting this to express that avoiding breaking changes is prioritized higher than having an actual working parts there. |
I certainly don't want to waste people's time. What kind of documentation would have helped you? The issue you referenced has a comment from me saying that we will not fix it until we decide to release v0.8.0 of tokio-util. |
I suggest documenting it right in the code. The path to discovery comes from an issue of trying to avoid a copy when you have a Concretely, I suggest researching an alternative that doesn't suffer from this issue and adding a documentation section to the code that references that alternative. Actually - I think there is a way to fix this - we can just add another type for the |
Also, I usually release things and bump versions when there is stuff to release - and this breaking change pretty much is that, so to me this justifies the release of In my view, the releases should follow the need to release the functionality, not the other way around. If want to hold the release of On a side note - |
I mean, it is probably about time for another breaking release of tokio-util. It's been two years since the last one. But it is not something I want to do often. Realistically, when I make a breaking change to tokio-util, that will result in many hundreds of other maintainers spending time on upgrading the version of tokio-util used in their projects. As for why tokio-util is not split up into several smaller crates, the reason is indeed maintenance. The Tokio project used to be split into many smaller projects in the v0.1 days. This was a very large maintenance burden and confusing for end-users, so the decision was taken to merge them into a smaller set of crates, which resulted in the set of crates you see today. The split between tokio and tokio-util exists because breaking changes are impossible in tokio, but not in tokio-util. |
Motivation
Fixes #6116
Solution
Implement
<'a> Encoder<&'a [u8]>
forLengthDelimitedCodec
.Call into
<'a> Encoder<&'a [u8]>
inEncoder<Bytes>
ofLengthDelimitedCodec
.