Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2018 edition requires adding xml-rs and log dependencies #22

Closed
steven-joruk opened this issue Sep 5, 2019 · 5 comments · Fixed by #106
Closed

2018 edition requires adding xml-rs and log dependencies #22

steven-joruk opened this issue Sep 5, 2019 · 5 comments · Fixed by #106

Comments

@steven-joruk
Copy link

steven-joruk commented Sep 5, 2019

I'm using 1.37 stable with 2018 edition enabled, and I found I needed to add the xml-rs and log dependencies in order to fix some build issues. This may just be my own misunderstanding of how rust dependencies work.

[package]
name = "minimum-yaserde"
version = "0.1.0"
authors = ["Me"]
edition = "2018"

[dependencies]
yaserde = "0.3"
yaserde_derive = "0.3"

# I wouldn't expect to have to add these myself
xml-rs = ""
log = ""

Without adding xml-rs as a dependency:

src/main.rs|6 col 10 error 433| failed to resolve: could not find `xml` in `{{root}}`
||   |
|| 6 | #[derive(YaDeserialize, Debug)]
||   |          ^^^^^^^^^^^^^ could not find `xml` in `{{root}}`
src/main.rs|6 col 10 error 433| failed to resolve: use of undeclared type or module `XmlEvent`
||   |
|| 6 | #[derive(YaDeserialize, Debug)]
||   |          ^^^^^^^^^^^^^ use of undeclared type or module `XmlEvent`

Without adding log as a dependency:

src/main.rs|5 col 10 error| cannot find macro `debug!` in this scope
||   |
|| 5 | #[derive(YaDeserialize, Debug)]
||   |          ^^^^^^^^^^^^^
@MarcAntoine-Arnaud
Copy link
Contributor

This is not linked to the 2018 edition.

But yes it's normal, you have to define dependencies for this kind of library as it generate some code for you automatically.
Required dependencies are defined here:
https://github.com/media-io/yaserde/blob/master/yaserde/Cargo.toml#L15

I'm not sure I can change anything here to automatically add the dependency in your library.

Marc-Antoine

@steven-joruk
Copy link
Author

Thanks, that makes sense - I'll look in to it as well.

@oscartbeaumont
Copy link
Contributor

@MarcAntoine-Arnaud couldn't you get around the need for these dependencies by exporting xml-rs and log from yaserde and then using the reexported version in the macro generated code.

I personally think that these dependencies should not be required in projects using yaserde to prevent conflicts with different versions of the dependencies and for a better developer experience.

Would you be interested in a PR using the solution stated above?

@MarcAntoine-Arnaud
Copy link
Contributor

Hello @oscartbeaumont ,

I'm interested too see a MR that can handle that !!!

Marc-Antoine

@oscartbeaumont
Copy link
Contributor

I created PR #115 using the solution I outlined above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants