Skip to content

Conversation

@HCastano
Copy link
Contributor

@HCastano HCastano commented Jul 11, 2022

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 a 4.0 pre-release
branch or something.

(Sorry about the formatting changes btw, looks like some of these files pre-date our use
of RustFmt)
I was on a different nightly, so my theory was wrong here

@HCastano
Copy link
Contributor Author

CI looks unnecessarily upset with some things 😅 I'll fix them tomorrow

@HCastano
Copy link
Contributor Author

@cmichi it looks like the estuary job runs with cargo-contract master, not the current HEAD for the PR - is that intended?

@HCastano HCastano requested a review from a team as a code owner July 14, 2022 21:37
@HCastano
Copy link
Contributor Author

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.

if let ink_metadata::MetadataVersioned::V3(ink_project) = ink_metadata {
Ok(ink_project)
let ink_metadata: ink_metadata::InkProject = serde_json::from_value(
Copy link
Collaborator

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.
@HCastano HCastano merged commit f903c03 into master Aug 24, 2022
@HCastano HCastano deleted the hc-versioned-metadata branch August 24, 2022 19:33
@ascjones ascjones mentioned this pull request Feb 14, 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.

4 participants