Skip to content

serde_derive #82

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

Closed
ArtemGr opened this issue Nov 1, 2016 · 15 comments
Closed

serde_derive #82

ArtemGr opened this issue Nov 1, 2016 · 15 comments

Comments

@ArtemGr
Copy link

ArtemGr commented Nov 1, 2016

Not sure if it's against the protocol, but I'd vouch for serde_derive to be added to the top crates.
I wanted to add a modern playground example to http://stackoverflow.com/a/37565736/257568 but it turns out there's no serde_derive there yet.

I mean, Serde is certainly in the top crates, but we'd want to promote the newer serde_derive version of the automation instead of the older serde_macros, right?

@dtolnay
Copy link
Member

dtolnay commented Feb 8, 2017

Also let's upgrade to serde 0.9.

@shepmaster
Copy link
Member

The selected crates are based on the top 100 crates by downloads, then taking the newest version of those and their dependencies. You can check out the exact logic, if so inclined.

Running the code as-is today, we would upgrade to:

serde = "0.9.6"
serde_codegen = "0.9.0"
serde_codegen_internals = "0.13.0"
serde_json = "0.9.5"

I might just go ahead and perform that update tonight.

But there's nothing that would cause serde_derive to get pulled in. I'm hesitant to start cherry-picking crates for inclusion based on my own whims. If I were to do that, I'd certainly blacklist xml-rs in favor of my own XML parser 😈

Additionally, I think about how I'd explain to someone why a given crate is available or not. If it's "someone submitted a PR", then I fear we'd be compiling 1000s of crates every night. If it's "the maintainer picked it", then I fear the backlash directed towards me on public (and private?) forums.

I would be interested in an improved algorithm that maybe could be more strongly influenced by recentness. Perhaps the most recent crates in the last 30-60-90 days would be better?

@dtolnay
Copy link
Member

dtolnay commented Feb 8, 2017

I understand, although serde_codegen is 100% deprecated and there is no way to use it correctly from the playground (nor has there ever been). It is replaced by serde_derive which would be easily usable.

@ArtemGr
Copy link
Author

ArtemGr commented Feb 8, 2017

I would be interested in an improved algorithm that maybe could be more strongly influenced by recentness. Perhaps the most recent crates in the last 30-60-90 days would be better?

Looking at the charts, serde_derive is much more popular today.
https://crates.io/crates/serde_macros is 400 downloads a day or something.
https://crates.io/crates/serde_derive is more like 2000.

Using last 90 days popularity seems like a good idea to me.

@shepmaster
Copy link
Member

there is no way to use it correctly from the playground

The current rationale is that if the playground has to spend time compiling a crate as a dependency, it's going to try as hard as possible to allow using that crate directly. Obviously, it doesn't work for all crates.

@shepmaster
Copy link
Member

Using last 90 days popularity seems like a good idea to me

Are you aware if there is an API endpoint for the data of "the top N crates, sorted by downloads in the last Y days"?

@dtolnay
Copy link
Member

dtolnay commented Feb 8, 2017

Would you be willing to compile crates that have a playground feature with that feature enabled for the purpose of dependency resolution? I think this resolves your concerns around explaining why a crate is available or not.

serde/Cargo.toml
[features]
playground = ["serde_derive"]

[dependencies]
serde_derive = { version = "0.9", optional = true }

@ArtemGr
Copy link
Author

ArtemGr commented Feb 8, 2017

Are you aware if there is an API endpoint for the data of "the top N crates, sorted by downloads in the last Y days"?

I guess it can be added to https://github.com/sebasgarcep/crates-api ?

@shepmaster
Copy link
Member

Would you be willing to compile crates that have a playground feature with that feature enabled for the purpose of dependency resolution? I think this resolves your concerns around explaining why a crate is available or not.

Ah, that's very interesting! In that case, we'd pull in serde for whatever reason (such as top-100 all-time downloads or an improved recent measure), and then serde could inform us what features are best used to demonstrate itself in the playground.

A technical question: do you know of a way of programmatically checking if a crate supports a feature, starting from only the name of the crate?

A social question: do you think that crate maintainers would be overall receptive to adding such a feature? I'm assuming that serde would, or else you might not have suggested it 😇. Is it ugly from the point-of-view of a maintainer, to customize your packaging for this arbitrary context?

@shepmaster
Copy link
Member

I guess it can be added to https://github.com/sebasgarcep/crates-api ?

I don't mind doing the HTTP / JSON myself, I just meant that I don't know that the crates.io server currently exposes such an endpoint.

@dtolnay
Copy link
Member

dtolnay commented Feb 8, 2017

do you know of a way of programmatically checking if a crate supports a feature, starting from only the name of the crate?

If you can get a Cargo.toml, the cargo metadata --no-deps command gives a list of the features declared in it. You can use the cargo_metadata crate to process it in a structured way.

do you think that crate maintainers would be overall receptive to adding such a feature?

Yes. I don't think this is a particularly unconventional use of cargo features. We already have things like "unstable-testing" for dependencies of our CI suite like clippy.

@dtolnay
Copy link
Member

dtolnay commented Feb 8, 2017

I opened serde-rs/serde#755 to add a playground feature on our end.

@dtolnay
Copy link
Member

dtolnay commented Feb 10, 2017

I opened #99 with an implementation of my proposed approach using a playground cfg.

@shepmaster
Copy link
Member

I've opened a new issue for potentially using recent crate downloads in #101

@shepmaster
Copy link
Member

And it's alive

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

No branches or pull requests

3 participants