Kriskras99 commented on PR #528:
URL: https://github.com/apache/avro-rs/pull/528#issuecomment-4974212230

   Hi @PookieBuns,
   
   I've finally had time to review this in depth. I think this is a good thing 
to add and it's almost at the point where we can merge it. I've pushed a commit 
which adds documentation and simplifies the code a bit.
   Unfortunately there are 3 tests (which you had marked as `should_panic`) 
which I think should work. The issue common to these 3 tests is an (union) enum 
with only one variant inside an Option. To the serializer code it looks like an 
Option so it takes the wrong branch at `serde/ser_schema/mod.rs:409`. As I 
think this is a common case (there is no serde trickery here with untagged or 
other attributes) it needs to be fixed before we can merge it (it would be 
confusing for the user if this case does not work).
   I'll therefore put it onto the 0.23 milestone and start thinking about how 
we can fix this without degrading performance for Options and without requiring 
more complex code.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to