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

this -> address(this) #1144

Merged
merged 2 commits into from
Oct 2, 2018
Merged

Conversation

BrendanChou
Copy link
Contributor

@BrendanChou BrendanChou commented Aug 2, 2018

Fixes #

Fixes errors in anticipation of Solidity 0.5.0

πŸš€ Description

  • πŸ“˜ I've reviewed the OpenZeppelin Contributor Guidelines
  • βœ… I've added tests where applicable to test my new functionality.
  • πŸ“– I've made sure that my contracts are well-documented.
  • 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).

@BrendanChou BrendanChou force-pushed the master branch 3 times, most recently from f45a52a to 1508187 Compare August 2, 2018 18:25
@nventuro
Copy link
Contributor

nventuro commented Aug 2, 2018

Hey @BrendanChou, thanks for this contribution!

I noticed there are some errors on our nightly build which seem related to this, we get these sorts of messages: TypeError: Return argument type contract SecureTargetMock is not implicitly convertible to expected type (type of first return variable) address.

Are those errors caused by this same issue, and if so, could you also address those? This is the nightly build for this PR, for reference: https://travis-ci.org/OpenZeppelin/openzeppelin-solidity/jobs/411415060

Thanks!

@nventuro nventuro added kind:improvement contracts Smart contract code. labels Aug 2, 2018
@nventuro nventuro self-assigned this Aug 2, 2018
@nventuro nventuro changed the base branch from master to solc-nightly October 2, 2018 12:54
@nventuro
Copy link
Contributor

nventuro commented Oct 2, 2018

I've updated the PR to target the solc-nightly branch.

@axic
Copy link
Contributor

axic commented Oct 2, 2018

This should be backwards compatible and could be merged on master.

@nventuro
Copy link
Contributor

nventuro commented Oct 2, 2018

@axic I'd rather have all of the 0.5.0 related changes into this branch, so that merging back to master is then as straightforward as possible.

Merging despite the build failing (for unrelated reasons).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Smart contract code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants