This repository has been archived by the owner on Jan 24, 2022. It is now read-only.
Trim ZWeb3 #1369
Merged
Trim ZWeb3 #1369
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frangio
commented
Dec 31, 2019
| it('returns checksummed address', function() { | ||
| ZWeb3.toChecksumAddress(accounts[0]).should.be.eq(accounts[0]); | ||
| }); | ||
| describe('checksum address', function() { |
There was a problem hiding this comment.
This is unrelated to the PR purpose but I was editing this file and noticed that this describe block was incorrectly placed within the previous describe block.
|
I don't consider this a breaking change, but I'm reluctant to removing many of the methods from ZWeb3 that would allow us to switch to a different library in the future (if we need to). Let's discuss it live. |
spalladino
approved these changes
Jan 6, 2020
| @@ -1,4 +1,6 @@ | |||
| export const ERC20_PARTIAL_ABI = [ | |||
| import { AbiItem } from 'web3-utils'; | |||
There was a problem hiding this comment.
I love how in every PR you keep adding more typings
frangio
pushed a commit
to OpenZeppelin/docs.openzeppelin.com
that referenced
this pull request
Feb 27, 2020
Raised in the forum by a community member: https://forum.openzeppelin.com/t/typeerror-zweb3-accounts-is-not-a-function/2343 ZWeb3 was changed in OpenZeppelin CLI 2.7 >https://github.com/OpenZeppelin/openzeppelin-sdk/releases/tag/v2.7.0 >Trimmed the API of the ZWeb3 object to remove methods duplicated from web3.js. (OpenZeppelin/openzeppelin-sdk#1369)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR trims ZWeb3 by removing all the functions which are just wrappers around web3 functionality. I additionally made
ZWeb3.etha getter (it used to be a function) so that the syntax for using it is nicer (without parentheses).For example
ZWeb3.accounts()has been removed and it now becomesZWeb3.eth.getAccounts().I've also removed many tests that were only testing the wrapped functionality, because they were just testing the web3 dependency. We might want to keep them though, given that web3 is kind of unstable, but IMO they would not be enough to detect any significant issues.
Do we consider this a breaking change?