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

ERC721 contracts no longer initialize their parents. #33

Merged
merged 1 commit into from Oct 15, 2018
Merged

ERC721 contracts no longer initialize their parents. #33

merged 1 commit into from Oct 15, 2018

Conversation

nventuro
Copy link
Contributor

@nventuro nventuro commented Oct 15, 2018

This fixes the issue with diamonds in the ERC721 inheritance tree (namely, having to pass name and symbol multiple times to MetadataMintable). Initializer for all parents have to be manually called (the mocks are an example of this).

Since we'll be providing a metadata, enumerable, mintable and pausable standalone ERC721 as part of our EVM package, this shouldn't be too bad, thought I'm not thrilled that initializers forced us to go this way.

I've also added some initialization safety checks in the style of #27.

@nventuro nventuro added this to the v2.0 milestone Oct 15, 2018
@nventuro nventuro merged commit 84a37e1 into OpenZeppelin:master Oct 15, 2018
@nventuro nventuro deleted the erc721-init branch October 15, 2018 17:32
@coveralls
Copy link

Pull Request Test Coverage Report for Build 76

  • 8 of 9 (88.89%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.8%) to 98.91%

Changes Missing Coverage Covered Lines Changed/Added Lines %
contracts/token/ERC721/ERC721Enumerable.sol 1 2 50.0%
Totals Coverage Status
Change from base Build 75: -0.8%
Covered Lines: 565
Relevant Lines: 566

💛 - Coveralls

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.

None yet

3 participants