Skip to content
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

Prepare 0.5.10 release #1603

Merged
merged 4 commits into from
Dec 30, 2021
Merged

Prepare 0.5.10 release #1603

merged 4 commits into from
Dec 30, 2021

Conversation

abonander
Copy link
Collaborator

@abonander abonander commented Dec 30, 2021

I got lazy when I wrote the changelog for 0.5.8 and didn't list all the PRs, and some folks were disappointed that their PR wasn't listed. So listing all 31 for this release cycle!

…est versions

chore(cli): upgraded `clap` to `3.0.0-rc.9`
I'm well aware of the principle that a spuriously failing test is a failing test, but even though I have it outputting the seed used with a reproducible PRNG, I can't reproduce the failures locally, so 🤷.
@abonander abonander marked this pull request as ready for review December 30, 2021 01:25
@abonander abonander merged commit fdbfc5d into master Dec 30, 2021
@abonander abonander deleted the ab/0.5.10 branch December 30, 2021 01:25
@05storm26
Copy link
Contributor

@abonander Oh, oh you have bumped up the indexmap version back to 1.7 from 1.6.2 ergo reverting this: #1501

This creates a dependency cycle bug: #1521

The bug is about the ahash dependency:
tkaitchuck/aHash#95

@05storm26
Copy link
Contributor

@abonander Could you downgrade the indexmap dependency to 1.6.2 again? Or is there a specific reason why sqlx needs 1.7 instead of 1.6.2?

@abonander
Copy link
Collaborator Author

Damn it. I ran cargo upgrade which I think is from cargo-edit just to get various dependencies caught up.

Why hasn't that been fixed upstream yet?

@05storm26
Copy link
Contributor

Its basically about wasm-bindgen and the maintainer @alexcrichton not having time for a major redesign :\

See this comment: tkaitchuck/aHash#95 (comment)

(and maybe also the 2 comments before it)

@abonander
Copy link
Collaborator Author

Yeah I just read back through the discussion. You'd think one of those other packages would have made a change to break the chain by now. Keeping a dependency pinned at a single version forever is not a solution.

@paolobarbolini
Copy link
Contributor

Damn it. I ran cargo upgrade which I think is from cargo-edit just to get various dependencies caught up.

Honestly I wish that wouldn't be done. It forces downstream users to upgrade all of their indirect dependencies when in some cases there might be a reason to stay back and it creates merge conflicts when dealing with PRs or forks (which for example we have to deal with as we have a fork with PRs targeting 0.6.0 already merged)

@abonander
Copy link
Collaborator Author

@paolobarbolini yeah, to be fair I mostly did it just to head off any complaints about outdated dependencies. There's not exactly a checklist for doing releases of crates of this size. Someone should write one.

@05storm26
Copy link
Contributor

@abonander Libs shouldn't need to worry about outdated dependencies because I think cargo uses the newest version that has the same major version as the specified one.

So writing 1.6.2 allows cargo to use 1.7 but not 2.1 for example.

Libs really shouldn't ever need to update their dependencies except for 2 cases:

  • There is something in the 1.7 version that is not in 1.6.2 and you need to use it.
  • There is a major version release of a dependency (like 2.0) and the community has started to migrate to that and you want your lib crate to also be compatible with that version and not needlessly use the old version.

If this is true cargo update really shouldn't even work for lib crates IMO. It should give some warning or something if run on a lib crate.

@paolobarbolini
Copy link
Contributor

to be fair I mostly did it just to head off any complaints about outdated dependencies

I understand, I'm subscribed to some projects and there's always someone thinking that the way to upgrade patch versions of their indirect dependencies is by sending a PR.

Maybe there should be a CI job checking that the minimum versions still work, and just reply to complaints and PRs with instructions for cargo update.

I'm also part of deps.rs (this is what it looks like for sqlx https://deps.rs/repo/github/launchbadge/sqlx), which until recently would complain very aggressively when the allowed version range included an insecure version of a dependency, but even then people would most of the times ignore it. From this it seems acceptable not to bump dependencies in Cargo.toml, unless something very serious happened obviously.

@abonander
Copy link
Collaborator Author

abonander commented Jan 12, 2022

It was cargo upgrade specifically, not cargo update.

And yes, I understand how dependency resolution in libraries works. For the record, this is the discussion I had in mind when I ran it: #1586 (comment)

@abonander abonander mentioned this pull request Feb 2, 2022
2 tasks
@abonander abonander mentioned this pull request Feb 23, 2023
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

Successfully merging this pull request may close these issues.

3 participants