-
Notifications
You must be signed in to change notification settings - Fork 95
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
Add some more fields. #61
Conversation
Please make all of the new fields |
|
||
/// A target platform. | ||
#[derive(Clone, Serialize, Deserialize, Debug)] | ||
pub struct Platform(String); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be #[serde(transparent)]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't understand the difference. What does transparent
do that the normal newtype struct doesn't do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For JSON there's no difference I think, but some data formats may include the name for unit-structs, which is exposed for non-transparent wrappers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. It also generates less code, which seems nice.
I'm a little confused. Isn't that the default behavior for |
I don't think so. But since you are mentioning it, a test would be great ;) |
I've updated with a new test suite. I can split that off into a separate PR, if you'd like. In the past I have missed having a way to test things, so maybe this can establish a way to ensure everything works. Hopefully it's ok? It actually helped exposed a bug in either cargo or cargo_metadata (rust-lang/cargo#6534 — I'm hoping to just fix it in Cargo). |
Absolutely wonderful! Thanks |
@oli-obk I'd appreciate a release 😉 |
ups, done |
This is to help assist with stabilizing alternative registries.
Dependency::rename
— Stabilized in 1.31.Dependency::registry
— Recently added in Add dependencyregistry
tocargo metadata
. rust-lang/cargo#6500Dependency::target
— Add typed as suggested in Expose remaining fields in Dependency #41 (comment). Parsing this is rather complex (see https://github.com/rust-lang/cargo/blob/dcb4360c50574483dc96e8862afcb1e581c7b95c/src/cargo/util/cfg.rs).Package::links
— Recently added in Addlinks
tocargo metadata
. rust-lang/cargo#6480The
registry
field could use a URL type (withurl
andurl_serde
crates) if you want it to be more strictly typed (ala #59).