search

sigp / superstruct

Rust library for versioned data types
https://sigp.github.io/superstruct/
Apache License 2.0
65 stars 3 forks source link

add flatten #34

Closed realbigsean closed 8 months ago

realbigsean commented 8 months ago

Resolves https://github.com/sigp/superstruct/issues/30

michaelsproul commented 8 months ago

awesome! will review tomorrow

realbigsean commented 8 months ago

fixed a little bug here, but now this is working in lighthouse https://github.com/sigp/lighthouse/pull/5158

Also a limitation right now is we don't have a way to nest inner types that have a subset of variants of the outer type. I think vice-versa should work though (haven't tested it). So nesting an execution payload in a block in lighthouse wouldn't benefit from this for example.

realbigsean commented 8 months ago

Also a limitation right now is we don't have a way to nest inner types that have a subset of variants of the outer type. I think vice-versa should work though (haven't tested it). So nesting an execution payload in a block in lighthouse wouldn't benefit from this for example.

This actually wasn't so bad to get working, I added it here: 76b67a19fdea613b5f7050e43644544f115aa517

michaelsproul commented 8 months ago

I pushed a bugfix for an issue I noticed with using the field_index to modify the variant_fields. If there are earlier fields that aren't on a variant (due to only), then the field index from the whole struct will not be the location of the field in the variant_fields. I added a test that fails without my change, and passes with it, but I could do with a second pair of eyes on it

https://github.com/sigp/superstruct/pull/34/commits/52eba9fb1df8bc09628c615dc6117aea01bf1e3a

michaelsproul commented 8 months ago

I also think we want to use the last part of the path, e.g. types::ExecutionPayload we want ExecutionPayload not types: https://github.com/sigp/superstruct/pull/34/commits/30f768f5206858f090b89a93ba02e4038ce0307f