Skip to content
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

Rename metadata.json to {contract_name}.json #950

Closed
athei opened this issue Feb 3, 2023 · 6 comments · Fixed by #952
Closed

Rename metadata.json to {contract_name}.json #950

athei opened this issue Feb 3, 2023 · 6 comments · Fixed by #952

Comments

@athei
Copy link
Contributor

athei commented Feb 3, 2023

I don't think it makes sense to name the contract and wasm file after the contract but not the metadata file. It is just annoying to not be able to see from the file name which contract it belongs to.

@kvinwang
Copy link
Contributor

kvinwang commented Feb 6, 2023

Bundled apps running on modern platforms often come with a manifest file, AndroidManifest.xml for Android apps for example.
So, I think the matadata.json for ink plays the same role. It is more friendly for external tools to have a static name. We have tools to read the contract name from the metadata.json.

@ascjones
Copy link
Collaborator

ascjones commented Feb 6, 2023

Bundled apps running on modern platforms often come with a manifest file, AndroidManifest.xml for Android apps for example. So, I think the matadata.json for ink plays the same role. It is more friendly for external tools to have a static name. We have tools to read the contract name from the metadata.json.

I'm sympathetic to this requirement, we don't want to introduce breaking changes unnecessarily.

I don't think it makes sense to name the contract and wasm file after the contract but not the metadata file. It is just annoying to not be able to see from the file name which contract it belongs to.

In some ways we have the best of both worlds at the moment, because the mycontractname.contract bundle is just metadata.json with the code embedded. It is a legacy thing for sure, but we should think carefully before breaking things and have a very strong reason to do so.

@SkymanOne
Copy link
Contributor

I agree with @kvinwang analogy with AndroidManifest.xml. There are services that take statically read the manifest files.

@athei
Copy link
Contributor Author

athei commented Feb 6, 2023

I can't help but thinking of:
https://xkcd.com/1172/

Should we name the other artefacts to static names, too? I am just wondering why it seems to be okay for them but not for the json file.

@ascjones
Copy link
Collaborator

ascjones commented Feb 7, 2023

Should we name the other artefacts to static names, too? I am just wondering why it seems to be okay for them but not for the json file.

No reason other than the legacy issue of metadata.json. Arguably it is a generated artefact rather than a manifest, and as such it is logical for it to have the name of the source.

My reasoning was purely that it is a breaking change, and we should at least think about it before doing it, as we are doing here.

Having thought about it, this is a major release 2.0.0, so this is a good place to do such a breaking change. Downstream clients which expect metadata.json can just assume that any *.json file is contract metadata file.

@athei
Copy link
Contributor Author

athei commented Feb 7, 2023

Exactly. This is the most trivial breaking change you can imagine. If we can't do this in a major release we are not able to change anything. Sorry @kvinwang. I know when stuff breaks this is annoying. But I think this will be the least of your problems when migrating to the new ink! version :)

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 a pull request may close this issue.

4 participants