-
Notifications
You must be signed in to change notification settings - Fork 9
Tracking issue: Major version bumps #222
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
Comments
Ah good! Yeah this is neat. I remember you talking a while ago about moving some deps to the |
On that same note, is there a way of checking deps are actually used? I want to say I’ve been very strict with that over the years, but there’s every possibility some of the larger crates are littered with graveyard deps in the first place… And then finally, there’s the matter of crate consistency. I think in some cases I might have switched crates halfway through - the logging thing is a good example. Might be worth to also sit down one day and try to homogenize that (e.g., make them all use the tracing thing or at least envlogger/humanlog, whichever you prefer) |
We can, not sure if we should, at least not for all packages. They are a bit more of a pain to work with than the normal way of declaring packages, but like you said, they do make it easier to be consistent on what version is actually used. Maybe it can be nice for packages that we use a lot throughout the workspace, but doing it for every dependency might be more hassle than it's worth. Edit: A small additional point against setting all packages via workspace dependencies would be that you would unnecessarily increase the minimal version, but I think this is so minor that it should not influence the decision either way. Just for completeness. The main pain arises from Dependabot that is just broken for Cargo. A solution like this would have my preference, it could just open a PR with the updated manifest, and if CI fails it is a clear indication that more work is necessary for that PR to be merged.
There are 3 ways of doing it that I know of. In ascending order of sophistication.
Reasonably, I found some using
Yeah, this is the biggest pain, because there exists no algorithm for such determination. We could sit down, or if you prefer, feel free to look through the manifests and reason one by one whether we have found a better alternative by now, I wish I could do it for you, but I haven't made the decision to switch so besides some obvious cases it is hard for me to determine if you used crates with enough overlap to reduce them to one. |
Ow, as in, for this workspace dependency thing you mean? Yes that'd be annoying. And you're right that only relatively new versions of Cargo support this feature. Still, for some deps (e.g.,
Oh, I forgot about that one! Well then probably we have the most of them.
Yeah of course not... Well, I'll keep an eye out for any I notice. And feel free to ask me of course if you find any suspects :) Nice that the dependency list is growing cleaner and cleaner though! 👍 |
No, just dependency management in general. The PRs I am creating are very trivial in basis. When a new major version is released, bump the manifest version to the lowest minor-patch version of the new major version. Most often the incompatibility is some other part of the API that you aren't even touching so nothing else needs to be done, but if Dependabot already does that manifest part and some problem occurs, it is really neat to add another commit to the branch in which you solve the issues in the usage. This would normally be the workflow I would recommend. However, Dependabot is quite broken when it comes to Cargo, requiring us limit updates to minor/patch versions and setting lockfile-only for them. Relevant issues: In conclusion, this is just some manual labour for the foreseeable future. (Or I might look into alternative like
Yeah, I think you are right, but more in a see as we go approach. It is very important if we have workspace members that must have the same version of a package in order to work correctly. This is mainly the case when a dependencies type leaks through the API. This is very common with transparent error types. (This also means any major update of a dependency is a breaking change, btw).
Will do |
That's... disappointing! Hmm. Maybe check
Fair point! Then it's extra nice to have them synced, yes. |
I can't remember where we talked about it (🌚) but I agree that the RE the |
We have a lot of dependencies, we also have a lot of dependencies which still need some major upgrades.
Here is a tracking issue to keep track of the upcoming major version updates.
I hope that this could lower our total dependency count a lot and therefore lower the compile times as well.
The relevant dependencies in decreasing order of usage:
reqwest
tonic
rand
base64
env_logger
prost
bollard
hyper
x509-parser
async-compression
thiserror
rustyline-derive
rustyline
openapiv3
getrandom
dashmap
dirs
graphql_client
rustls
tokio-rustls
rustls-pemfile
nom_locate
nom
scylla
Dependencies with overlapping functionality:
Using
cargo upgrade --dry-run --compatible ignore --incompatible allow
we find what needs to be done.I am going to do this on a dependency basis, not on a workspace member basis, as I suspect the fixes are probably pretty repetitive.
The text was updated successfully, but these errors were encountered: