Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't lock the version. Locking would be
=0.19.8
. However, I'm not sure this is required here.The problem is that
toml_edit
bumped the MSRV in a patch release. The MSRV is also computed based on all deps. Meaning that the MSRV ofproc-macro-crate
is still1.60.0
, but it gets transitively bumped to1.64.0
because of its deps.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your MSRV is at least the MSRV of your dependencies. I'd appreciate if we can lock the dependency here to avoid cascading the breaking change up to your users.
proc-macro-crate
is a dependency ofparity-scale-codec
which is integrated intomultihash
.multihash
is a fundamental crate of the libp2p ecosystem and we are trying to minimize breaking changes there at the moment. We treat bumping MSRV as a breaking change.For 1.0 of
multihash
, I want to avoid the dependency onparity-scale-codec
, simply because it is unnecessarily part of the public API. It doesn't need access to the internals and can be implemented on top. We've chatted about this in paritytech/parity-scale-codec#405 already :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked to pin
0.19.8
, but that doesn't help. The problem is thattoml_edit
itself doesn't haswinnow
pinned so cargo will still use the latest version.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can build this crate with older Rust versions still. There are no changes needed here, but your application that wants to be able to build with older Rust versions will have to come with a suitable
Cargo.lock
that pinstoml_edit
to an old enough version.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More importantly, if you pin it in this crate's
Cargo.toml
to==0.19.8
you will break dependency resolution in other crates depending on this, and which actually require features fromtoml_edit > 0.19.8
. It's generally a bad idea to have upper bounds inCargo.toml
(or even worse exact versions).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, I guess we should just downgrade
toml_edit
in CI before trying to compile.