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

Dependency cleanup #37

Merged
merged 3 commits into from Nov 25, 2019
Merged

Dependency cleanup #37

merged 3 commits into from Nov 25, 2019

Conversation

mathstuf
Copy link
Contributor


Is it worth trying to find the actual minimum version of serde for toml-rs? See toml-rs/toml-rs@381d020 for details on that change.

This release made the `Color` type `impl Copy` which is used in this
crate.
This fixes `-Z minimal-versions` builds because this version of `toml`
requires sufficient versions of its dependencies.
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Would it be possible to continue using the serde derive macros through the serde::{Deserialize, Serialize} reexport? That is the approach recommended in https://serde.rs/derive.html.

@mathstuf
Copy link
Contributor Author

No, because serde's minimum version is 1.0.0 and in a -Z minimal-versions build, 1.0.0 ends up not being suitable for what is used. See serde-rs/serde#1647.

@mathstuf
Copy link
Contributor Author

Oh, it would be. However, this would leave serde_derive as an unused dependency in this crate.

@dtolnay
Copy link
Owner

dtolnay commented Oct 28, 2019

I maintain Serde as well so it feels like we should fix it there. Fixing this this way in this crate doesn't seem like the right fix. Could you send a PR to serde instead?

The documentation for `serde`'s `derive` feature states that it should
really only be used for libraries with an optional `serde` feature
themselves. That's not happening here.

However, the need to split it is also required by `-Z minimal-versions`
builds. Without a direct dependency, `serde_derive` 1.0.0 gets chosen.
This is not suitable for this crate because it uses the `alias` feature.
1.0.103 is selected because it is the version that is compatible with
`-Z minimal-versions`.
@mathstuf
Copy link
Contributor Author

mathstuf commented Nov 1, 2019

Now depends on a release of serde-rs/serde#1664

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay dtolnay merged commit f48fc4a into dtolnay:master Nov 25, 2019
@dtolnay
Copy link
Owner

dtolnay commented Nov 25, 2019

Published in 1.0.18.

@mathstuf mathstuf deleted the dependency-cleanup branch November 25, 2019 14:00
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 this pull request may close these issues.

None yet

2 participants