-
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
Add support for ink!'s version
metadata field
#641
Conversation
CI looks unnecessarily upset with some things 😅 I'll fix them tomorrow |
@cmichi it looks like the estuary job runs with |
The estuary job can't pass until we stop using Git dependencies for the ink! dependencies (can't publish a crate that includes ink! dependencies). |
There's only one metadata version that can be constructed, so the check doesn't totally make sense. In the future if we support both V4 and V5 metadata formats it would make sense to add such a check back.
transcode/README.md
Outdated
|
||
if let ink_metadata::MetadataVersioned::V3(ink_project) = ink_metadata { | ||
Ok(ink_project) | ||
let ink_metadata: ink_metadata::InkProject = serde_json::from_value( |
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.
Nice that you fixed this example in the README
, however it is quite brittle for the code to be here in the first place. See #705
There's only one metadata version that can be constructed, so the check doesn't totally make sense. In the future if we support both V4 and V5 metadata formats it would make sense to add such a check back.
Related ink! PR: use-ink/ink#1313.
This can't be merged until the ink! PR is merged.
We may also want to replace the Git references with a
patch
to a4.0
pre-releasebranch or something.
(Sorry about the formatting changes btw, looks like some of these files pre-date our useI was on a different nightly, so my theory was wrong hereof RustFmt)