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

Feature: Fetch metadata Browser-Harmony #8

Merged
merged 7 commits into from
Jan 17, 2021
Merged

Feature: Fetch metadata Browser-Harmony #8

merged 7 commits into from
Jan 17, 2021

Conversation

RolandoAndrade
Copy link
Contributor

Why is this change neccesary?
Because is neccesary retrieve the ERC721 contract metadata from the browser.

How does it address the issue?
I created an interface for contract manipulation at contacts/domain where I defined the ERC721Contract with the method getMetadata(), in the future here will be defined all the methods that we will use, the idea is manipulate the contracts in an abstract way, so we can use the same procedures at front applying polimorfism.

The implementation of the abstract contract is ERC721HarmonyContract located at contracts/infrastructure, here I put all the Harmony logic and contract wrappers. I created an instance of it as example at index.js. If we use polimorfism, we could instance an ERC721EthereumContractwithout change anything in the code, only the line where it is instanced.

What side effects does this change have?
There are a lot of changes in dependencies that must be installed. I deleted the dist file generated by webpack build process because it changes frecuently and could create git conflicts. On the other hand, I added typescript, but my configuration allows Javascript, so the only side effect that could be appear is a conflict in build, and if you delete the dist file it won't appear.

Copy link
Contributor

@jcstr jcstr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @RolandoAndrade for this! I'll merge because we need it for the whole project.

@jcstr jcstr merged commit 1b3c945 into master Jan 17, 2021
@jcstr jcstr deleted the fetch-metadata branch January 17, 2021 19:03
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.

3 participants