search

media-io / yaserde

Yet Another Serializer/Deserializer
MIT License
175 stars 57 forks source link

Removing log and xml re-exports is a breaking change. #135

Open ephraimkunz opened 2 years ago

ephraimkunz commented 2 years ago

While building Yaserde from the git repo instead of the crates.io dependency (that worked fine), my project's build was broken.

That's because a commit on the master branch but not yet in a release removed the re-exports of the log and xml. I was using yaserde::xml in a custom implementation of yaserde::YaSerialize for a struct in my project.

As noted in the comment in #120 (which made this change), this is a breaking change. It seem like it should either be reverted in case other people are using the re-exports (which is convenient when writing custom implementations of YaDerialize / YaDeserialize) or it should be mentioned in the release notes so people know it's a breaking change.

I favor reverting the change that removed these exports because with things as they currently are, it's a little inconvenient to have to add the xml dependency manually and make sure to keep it as the same version that Yaserde uses internally.

ephraimkunz commented 2 years ago

I'm happy to try to figure out how to revert this if that's the route we want to take, or maybe @Felerius could take a look since he made the change initially.

luca-della-vedova commented 1 year ago

:+1: for reverting, it is not currently possible to implement a custom YaSerialize without either adding a dependency on xml manually or relying on a hidden import (changing yaserde::xml to yaserde::_xml) which, given that the API is not public, it has no assurance of being stable.