nabijaczleweli / cargo-update

A cargo subcommand for checking and applying updates to installed executables
MIT License
1.22k stars 42 forks source link

Simplify parsing of crate versions #34

Closed dtolnay closed 7 years ago

dtolnay commented 7 years ago

I get lost when I see things like Semver::parse(j["vers"].as_str().unwrap()).unwrap().

nabijaczleweli commented 7 years ago

What you've done there is you've pulled another, like, 2-3 crates in and made a structure you deserialise into, then out of? What? How does that simplify anything at all?

dtolnay commented 7 years ago

Thanks for the explanation @nabijaczleweli! I obviously disagree but I would love to understand your perspective better if you have the patience to explain.

You mentioned that there is a struct we are deserializing into then out of, but notice how the original code actually does much worse than that:

So that is one aspect of what I meant by simpler. The other aspect has to do with error conditions:

Literally that's it. The struct is a way to declare what we expect the input data to look like, and if it doesn't look like that, then that is the only way anything can fail (on line 284).

Yes serde_json has 3 new dependencies (not counting serde because you already depend on that) but all of them are extremely small. I understand minimizing dependencies so please be honest if that is the primary reason for deciding a particular way.