-
Notifications
You must be signed in to change notification settings - Fork 203
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
Remove duplicate dependencies in Cargo.toml #459
Comments
Issues:
|
Removed about 15 dependencies in https://github.com/jyn514/docs.rs/tree/versions. The rest of the duplicates are waiting on the issues above. |
This allows docs.rs to only have one version of toml, not both 0.4 and 0.5. See rust-lang/docs.rs#459 for context.
Reqwest and cargo have a lot of duplicated dependencies when combined, but not separately, probably because we're installing cargo from git and it uses a lot of latest version. Not sure there's a good solution for that. |
By the way, it would be ideal to completly remove the usage of |
Wow, it turns out cargo was completely unused! That just knocked off 40 dependencies at absolutely no cost :) |
Upgrade steps:
|
If we migrate stuff interacting with the database I'd like to see tests that check the serialized string doesn't change (landing the tests before in a separate PR).
Well, if we create a wrapper type like: struct JsonCompat(rustc_serialize::json::Json); ... we can manually
To be honest, do we really need that dependency? I just rolled out it myself for Crater, and it's like 30 lines of code. Especially if it's holding back our migration efforts I'd consider removing it completly. |
Good call, I can pull a few of the bigger examples out of the DB as test cases.
Would it make sense to convert all rustc_serialize structures to serde's Json directly? I think that would be about the same effort as a wrapper.
Ok great! We can just reuse that code then, I'm always in favor of getting rid of dependencies. |
Well, it depends if we want to do the switch across the whole application at the same time or gradually. I'd sort of prefer gradually... |
This is no longer an issue, see SkylerLipthay/schemamama_postgres#7 (comment) |
Update: @Kixiron tried to work on it and it goes down a very deep rabbit hole:
|
I think I've come up with a good combined solution, actually. Like Pietro's earlier solution, I think I'm going to make a |
Couldn't you use
I still prefer that we use derives whenever possible. |
Yah, I figured it all out in #709, there was some weirdness but I got it |
Once #910 is merged we can finally make progress on this 🎉 These are the duplicates from #910:
There are 90 in total, so 45 crates we can remove altogether.
Various other issues. Will deal with them more once tokio 0.2 lands. |
Most of the remaining duplicates come from |
Update: updating iron doesn't help, even 0.6 depends on an old version of hyper. We'd have to fork it entirely, which I'd rather not do. |
looking through old issues I stumbled onto this one. Is this something we still should invest time on? |
I noticed while building from the latest change to the Dockerfile that many dependencies are repeated with different versions. It would be nice to make sure that we only depend on 1 version of any given package so developers don't have to compile as much on their first build. Fixing this shouldn't be hard but it might be tedious.
Here's a graph of the duplicated dependencies, created with
cargo tree --duplicate
. Note that they are listed in reverse dependencies order, so the first duplicate here isaho-corasick
, which is required byregex
, which is required byhandlebars
, which is required byhandlebars-iron
.Duplicates
The text was updated successfully, but these errors were encountered: