-
Notifications
You must be signed in to change notification settings - Fork 120
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
Bump ink_*
crates to v3.3.1
#686
Conversation
[[package]] | ||
name = "ink_env" | ||
version = "3.0.0-rc8" |
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.
ayy lmao
ink_metadata = { version = "3.3", default-features = false, features = ["derive"] } | ||
ink_env = { version = "3.3", default-features = false } | ||
ink_storage = { version = "3.3", default-features = false } | ||
ink_lang = { version = "3.3", default-features = false } |
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.
What is your thinking here? This way the CI for cargo-contract
will no longer alert us if we merge something into ink! master
that breaks the linter.
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 crate was introduced before the ink! 3.0 release, so pulling Git deps looks like a remnant of that.
More importantly though, the lockfile is using old RC deps, which is what ends up getting used in the test-dylint
CI step, not ink! master
.
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 would leave it to reference the ink! master
, otherwise we might publish an ink! release that in the worst case breaks the last cargo-contract
release. We would only notice after having published the ink! release.
For the lockfile: Hmm, the CI is not using it. I guess we committed it to have --locked
working and forgot to update it.
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.
It seems wrong to test it in that way. If you want to test for that it should be a CI job on the ink! repository. Each project should check their own master against released versions of other crates.
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'm with Alex here. We'd see the linter breaking in the examples-contract-build
step of the ink! CI anyways
Updates all the
ink_*
dependencies instead of just a couple (take that dependabot!).Supersedes #684 and #685.