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

Remove duplicated constructor call in FinalizableCrowdsaleImpl #613

Merged

Conversation

federicobond
Copy link
Contributor

@federicobond federicobond commented Dec 15, 2017

This is going to be enforced by the compiler soon.

@shrugs
Copy link
Contributor

shrugs commented Dec 17, 2017

Can you give context for this?

@federicobond
Copy link
Contributor Author

Yes! See here.

@frangio
Copy link
Contributor

frangio commented Dec 22, 2017

Thanks @federicobond. 🙂

I wouldn't see this as duplicated super constructor calls. I find it somewhat weird because of the Solidity inconsistency between zero-arity constructors, which are implicitly called, and non-zero-arity constructors, which aren't.

Regardless, I'm all for removing the redundant constructor call in this case.

Can you rebase before merging?

@federicobond federicobond force-pushed the remove-duplicate-constructor branch from 057ef7f to 3871ae2 Compare December 24, 2017 03:56
@federicobond
Copy link
Contributor Author

Rebased. I agree with the inconsistency in zero-arity constructors. I'll bring it up for discussion in the Solidity repo.

AugustoL
AugustoL previously approved these changes Dec 28, 2017
@federicobond
Copy link
Contributor Author

Rebased.

Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Thanks @federicobond!

@frangio frangio merged commit a74b7bd into OpenZeppelin:master Jan 2, 2018
@frangio frangio removed the backlog label Jan 2, 2018
@federicobond federicobond deleted the remove-duplicate-constructor branch January 2, 2018 19:53
ProphetDaniel pushed a commit to classicdelta/Smart-Contracts that referenced this pull request Mar 9, 2018
…e-constructor

Remove duplicated constructor call in FinalizableCrowdsaleImpl
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.

5 participants