rust-ux / uX

Non standard integer types like `u7`, `u9`, `u10`, `u63`, `i7`, `i9`
Apache License 2.0
119 stars 37 forks source link

feat: borsh serde support of unsigned #74

Closed dzmitry-lahoda closed 2 months ago

dzmitry-lahoda commented 2 months ago

unsinged in next pr

bbaldino commented 2 months ago

I think the tryfrom stuff should be split out into its own PR, let's start with that. I don't know much about borsh--I've only used serde--but I can definitely see how serde support would be interesting, so supporting borsh seems reasonable.

dzmitry-lahoda commented 2 months ago

I don't know much about borsh

that why I documented it for you in one line :) it is pretty stupid byte level protocol, unlike protobuf.

but I can definitely see how serde support would be interesting, so supporting borsh seems reasonable.

when I take u64, i know it will serde automagically by prost, serde, borsh, bincode, etc. so out of box support is must if to pursue project goal. sure one could have say let these projects add uX and do serde, but that is not option until uX is popular, and uX is no option until it has serde.

dzmitry-lahoda commented 2 months ago

I think the tryfrom stuff should be split out into its own PR,

I know that is in general good practice, so could we avoid that in this case? Especially this case required adding all try from, so changes are related. And all was tested. So not imho in this case split is waste of time.

I can donate coffee not to do that :)

I have limited time and lot to do.

Need serde, protobuf, core::ops and num-traits, so hope for some resonable fast tracking imho.

dzmitry-lahoda commented 2 months ago

I know deeply know many serde formats and and wrote custom, so can make them right things fast if given chance to do so without friction.

bbaldino commented 2 months ago

I know that is in general good practice, so could we avoid that in this case? Especially this case required adding all try from, so changes are related. And all was tested. So not imho in this case split is waste of time.

I can donate coffee not to do that :)

I have limited time and lot to do.

Borsh may need the tryfroms, but the tryfrom impls are independent and a good thing on their own, so personally I would not merge these two in one PR. I understand if you don't have the time to do the split--that's a decision for you to make.