eclipse-zenoh
move Publisher.write method into trait
move Publisher.write method into HasWriteWithSampleKind trait (closes #731 )
The very reason for the write method to exist is a niche case of Stream-like interface where a subscriber can be piped into a subscriber. The very existence of write already generated some confusion on what's the core API of a publisher and what is available in other bindings. I believe we should separate it in a different trait, we could then argue about the naming...
To me, separating it doesn't make the API any simpler, so I find the cost of an API break to high.
Splitting a method off into a trait makes sense when there may be multiple implementors, or when adding methods to a foreign type. I don't think it's helpful to "hide" methods, especially given that this one's semantic is far from complex.
My reasoning is that this has no benefits (adding indirections in the doc is not one IMO), so there's no reason to break this API.
A much greater improvement IMO would be to change the doc to something like:
[`put`](Publisher::put) or [`delete`](Publisher::delete) based on the [`SampleKind`].
An example of this being useful is forwarding received [`Samole`]s without checking whether it's a put or a delete.
This would make the use-case and alternatives much easier to understand than sweeping the API under the rug.
Again, my point is that method does NOT exist and it makes the API asymmetrical. This an API whose sole reason to exist is for some Rust ergonomics. I tend to disagree with Splitting a method off into a trait makes sense when there may be multiple implementors, it can also be use to explicitly group a set of methods under a given trait.
What we want to convey here is: this is an API specific to Rust, not a generic Zenoh Publisher. Another way to see it, is that this API should implement something like StreamExt. I'm not sure we have the bandwidth to do a full StreamExt implementation for the publisher and a subscriber right now...
I'd support @p-avital. I'm also kind of against to the idea of using traits as a way to separate API to "first-class" and "second-class" elements.
At this moment we made a list of "core" api elements (https://github.com/eclipse-zenoh/zenoh/blob/tagging/api_core.txt), so the case of write is not unique. We have multiple functions which are part of public rust API, but not part of core API. So I see no problem to leave the writein publisher impl.
Write accepts a SampleKind and a Value, which is a duplication of already existing put and delete operations on the publisher. Is there any real usage for that or is it just a redundant API with not much added value? I'd be more in favour now to remove it.
Agree with @Mallets. The delete operation is not normally supposed to contain payload. So it's better to not expose the ability to make it, even if protocol can do it now