specify recent once_cell version in a single place #2350
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2350 +/- ##
=======================================
Coverage 94.89% 94.89%
=======================================
Files 103 103
Lines 24274 24274
=======================================
Hits 23034 23034
Misses 1240 1240 ☔ View full report in Codecov by Sentry. |
| @@ -67,7 +67,7 @@ log = { version = "0.4.8" } | |||
| macro_rules_attribute = "0.2" | |||
| mio = { version = "1", features = ["net", "os-poll"] } | |||
| num-bigint = "0.4.4" | |||
| once_cell = { version = "1.16", default-features = false, features = ["alloc", "race"] } | |||
| once_cell = { version = "1.20.3", default-features = false, features = ["alloc", "race"] } | |||
There was a problem hiding this comment.
Yes but I would personally downvote that idea.
I would personally vote against supporting too many outdated dependencies unless there is some kind of strong and documented motivation for this, supporting older MSRV could be an interesting example.
Outdated dependencies may have some bugs, potentially even security issues, that have already been resolved. Also I do not recall there is CI testing with the older dependency versions.
Leaving this decision up to you. I started this as a draft until I had the second commit ready. I would be happy to squash if needed.
There was a problem hiding this comment.
I suggest keeping this as "1.16". The lowest we can write here is "1.7" (introduction of the "race" feature).
Generally the minor number should be moved forward only when a specific API is required; this gives cargo the widest possible set of versions to select from. And the patch number should only be specified if we have a known, specific requirement on a fix it introduced.
There was a problem hiding this comment.
If we really want to sweat the details on this, we'd use cargo-minimal-versions to find (and then potentially test in CI) that we depend on the minimal version.
I don't think we should unnecessarily bump the requirement.
There was a problem hiding this comment.
There was a problem hiding this comment.
But I guess that only checks in one direction: whether the required version is still good enough. The more ambitious version would be to also do the bisection to figure out the oldest version that still allows us to pass the test suite.
| @@ -40,7 +40,7 @@ brotli-decompressor = { workspace = true, optional = true } | |||
| hashbrown = { workspace = true, optional = true } | |||
| log = { workspace = true, optional = true } | |||
| # only required for no-std | |||
| once_cell = { version = "1.16", default-features = false, features = ["alloc", "race"] } | |||
| once_cell = { workspace = true, default-features = false, features = ["alloc", "race"] } | |||
There was a problem hiding this comment.
I think the , default-features = false, features = ["alloc", "race"] can be dropped here?
| @@ -67,7 +67,7 @@ log = { version = "0.4.8" } | |||
| macro_rules_attribute = "0.2" | |||
| mio = { version = "1", features = ["net", "os-poll"] } | |||
| num-bigint = "0.4.4" | |||
| once_cell = { version = "1.16", default-features = false, features = ["alloc", "race"] } | |||
| once_cell = { version = "1.20.3", default-features = false, features = ["alloc", "race"] } | |||
There was a problem hiding this comment.
I suggest keeping this as "1.16". The lowest we can write here is "1.7" (introduction of the "race" feature).
Generally the minor number should be moved forward only when a specific API is required; this gives cargo the widest possible set of versions to select from. And the patch number should only be specified if we have a known, specific requirement on a fix it introduced.
No description provided.