search

paritytech / parity-scale-codec

Lightweight, efficient, binary serialization and deserialization codec
Apache License 2.0
247 stars 95 forks source link

Seal `HasCompact` + `CompactLen` #514

Open bkchr opened 1 year ago

bkchr commented 1 year ago

Both traits should probably not be implemented by anyone downstream. We should check again, but the can be very likely be sealed.

bkchr commented 1 year ago

We should check the usage in ruint and then decide on what to do. Maybe HasCompact could also gets a better name :see_no_evil: SupportsCompact? Or whatever. Maybe the entire trait structure is to complicated there and requires some rethinking.

koute commented 1 year ago

Maybe the entire trait structure is to complicated there and requires some rethinking.

This is just my personal unsubstantiated personal opinion, but I do think we kinda have too many traits (this crates exports 20 traits in total!) that make things a little too complicated, so if we're going to do a breaking change release it'd be nice to redesign the API a little bit.

Of course this would be significantly more work than just patching things along in a backwards-compatible manner as we usually have done up until this point.

bkchr commented 1 year ago

I mean we already had some breaking changes ;) Otherwise we would not have ended at 3.x. If you want to redesign it, do it :P

koute commented 1 year ago

Yeah, I know. :P I don't know if I have the strength to do the redesign right now (but at least with the monorepo in place for Polkadot updating everything would probably not make me want to immediately want to gouge my eyes out), just wanted to voice my three cents. I'm not going to block 4.0 as-is or anything like that. :P (If the only breaking change is the one we had then the changes required downstream should be relatively minimal anyway.)