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

test: add tests for max64 and min64 from Math #601

Merged
merged 2 commits into from
Jan 15, 2018
Merged

test: add tests for max64 and min64 from Math #601

merged 2 commits into from
Jan 15, 2018

Conversation

come-maiz
Copy link
Contributor

No description provided.

@come-maiz
Copy link
Contributor Author

I wanted to learn more about solidity, and contribute a little bit. So I started with the most basic missing test I could find :)

There's one thing I don't really understand. I copied everything from the SafeMath tests, and instead of returning the values, the functions in the mock save them in the result attribute. Is there a reason for that?

@eternauta1337 eternauta1337 added review good first issue Low hanging fruit for new contributors to get involved! and removed review labels Jan 2, 2018
@federicobond
Copy link
Contributor

Thanks @ElOpio! 🙌

The result state variable came from the time SafeMath was a base contract. Not sure why it was done that way. Perhaps @maraoz can shed some light.

@frangio
Copy link
Contributor

frangio commented Jan 15, 2018

Not sure either, but I think we can keep it this way and refactor later.

@frangio frangio merged commit 462c52b into OpenZeppelin:master Jan 15, 2018
@come-maiz come-maiz deleted the test/math branch January 15, 2018 21:36
ProphetDaniel pushed a commit to classicdelta/Smart-Contracts that referenced this pull request Mar 9, 2018
test: add tests for max64 and min64 from Math
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Low hanging fruit for new contributors to get involved!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants