[API Break] Make control::property::Info::value_type return & instead of clone
#217
Conversation
|
It was probably intended as a cheap Copy type, except that the presence of EnumValues (with Vecs) no longer makes that cheaply possible? |
|
Yeah, I was wondering about that, but haven't check the history. If it were a |
|
I think this is a good change, is this ready for merge? |
|
I think the only reason I marked this as a draft is because it's a breaking change. So if there's going to be a breaking release, it should be mergable. |
|
Didn't we already land a bunch of breaking changes? This can probably be merged after a rebase, I don't see the CI being unhappy with it after that :) |
|
yeah, I think this just needs a rebase |
It seems odd for this to return an owned value, when `::name()` doesn't. And it is useful to get a reference to the `ValueType`, so `ValueType::convert_value` can be used and just use the lifetime of the `Info` instead of the cloned `ValueType.` I assume there's no reason not to do this, since callers can still clone as needed.
|
CI passing, after also updating a line in |
|
thx! |
3 participants
It seems odd for this to return an owned value, when
::name()doesn't. And it is useful to get a reference to theValueType, soValueType::convert_valuecan be used and just use the lifetime of theInfoinstead of the clonedValueType.I assume there's no reason not to do this, since callers can still clone as needed.
As a small API break, I think this should be merged whenever the next semver bump to the crate happens to be.