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

Removes hardcoded contract ABIs #837

Closed
wants to merge 2 commits into from
Closed

Removes hardcoded contract ABIs #837

wants to merge 2 commits into from

Conversation

eternauta1337
Copy link
Contributor

Fix #818

While doing this, I saw that there are a lot of manual new web3.ethContract(abi, address) usages. I suggest that we open a new issue to add a "getContract" helper, similar to what already exists in utils.js but capable of retrieving a full Web3 Contract instance.

@eternauta1337 eternauta1337 requested a review from 0xGabi October 24, 2019 18:42
@eternauta1337 eternauta1337 self-assigned this Oct 24, 2019
@macor161 macor161 mentioned this pull request Oct 26, 2019
4 tasks
Copy link
Contributor

@macor161 macor161 left a comment

Choose a reason for hiding this comment

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

Nice work! 💯Just one or two small comments.

@@ -45,6 +45,7 @@
"@aragon/apps-shared-minime": "^1.0.1",
"@aragon/aragen": "^5.0.0",
"@aragon/cli-utils": "^0.0.8",
"@aragon/id": "^2.1.0",
"@aragon/truffle-config-v4": "^1.0.0",
Copy link
Contributor

@macor161 macor161 Oct 29, 2019

Choose a reason for hiding this comment

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

We should probably add the @aragon/os dependency here

Suggested change
"@aragon/truffle-config-v4": "^1.0.0",
"@aragon/os": "^4.3.0",
"@aragon/truffle-config-v4": "^1.0.0",

@0xGabi
Copy link
Contributor

0xGabi commented Oct 29, 2019

@ajsantander this PR have a lot of duplications from #833

I think we should merge them or decide which one to include.

Note: I like the name standardization that @dapplion included. Probably we want to keep that part.

@macor161
Copy link
Contributor

We can close this PR as it will be replaced by #833
Thanks for your contribution @ajsantander ! 🙌

@sohkai sohkai deleted the unified-abis branch November 5, 2019 00:51
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.

Import ABIs from @aragon/os and @aragon/id instead of hardcoding them
3 participants