-
Notifications
You must be signed in to change notification settings - Fork 33
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
Adding type and abis #122
Adding type and abis #122
Conversation
didToken: string, | ||
contractAddress: string, | ||
contractType: 'ERC721' | 'ERC1155', | ||
web3: Web3, |
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.
Curious, why web3 needs to be a param in this case?
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.
Just a thought, maybe web3 can be a peer dependency, which can keep our admin sdk light-weight and thin
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.
I made it a param to make the mocks more straightforward in the unit tests, and also to allow the users to pass in web3 instances that may have custom options.
I'm not familiar with peer dependencies, looking into it
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.
moved into peer dependencies @Ethella
β¦pendency-versions-to-support-web-3 Pdeexp 594 update admin sdk dependency versions to support web 3
π PR was released in |
π¦ Pull Request
β Fixed Issues
π¨ Test instructions