add missing visit_enum #2836
add missing visit_enum #2836
Conversation
|
@dtolnay is there an issue with this request? |
| let (key, data) = tri!(visitor.variant::<String>()); | ||
| Ok(Content::Map( | ||
| [( | ||
| Content::String(key), | ||
| tri!(data.newtype_variant::<Self::Value>()), | ||
| )] | ||
| .into(), |
There was a problem hiding this comment.
It seems that you should at least use Content instead of String, and probably introduce a new variant in Content for enum.
Also, need an explanation, why you choose newtype_variant? If that the only thing that expected when deserializing untagged and internally tagged enums, then this should be explicitly explained, because this is not obvious at all.
The last, but not least, you should add tests that cover this change. I tried to bring some hierarchial structure to the serde tests so that the new contributors know where to start, but not all PRs are accepted yet. However, for that part tests seems, already concentrated in one place:
I think that adding those details will increase chance of the PR to be reviewed by maintainers.
fixes surrealdb/surrealdb#4844 and probably dtolnay/serde-yaml#344