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

Initialize MAX_UINT more neatly #1166

Merged
merged 1 commit into from
Aug 8, 2018
Merged

Conversation

barakman
Copy link
Contributor

@barakman barakman commented Aug 8, 2018

Use 2 ^ 256 - 1 instead that huge constant value.

Fixes #

🚀 Description

Initialize MAX_UINT in a manner which makes it clear why this is indeed the maximum uint value.

  • [x ] 📘 I've reviewed the OpenZeppelin Contributor Guidelines
  • [x ] ✅ I've added tests where applicable to test my new functionality.
  • [x ] 📖 I've made sure that my contracts are well-documented.
  • [x ] 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).

Use 2 ^ 256 - 1 instead that huge constant value.
Copy link
Contributor

@nventuro nventuro left a comment

Choose a reason for hiding this comment

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

Wow, I wonder how I missed this, thanks @barakman!

@nventuro nventuro merged commit a3e02be into OpenZeppelin:master Aug 8, 2018
@barakman
Copy link
Contributor Author

barakman commented Aug 8, 2018

@nventuro:

You're welcome.
I've got a few more of those (mostly submitted by now).
Up until now, I used to fix them locally on my repo, but it's becoming harder and harder for me to merge changes from OpenZeppelin repo.

@nventuro
Copy link
Contributor

nventuro commented Aug 8, 2018

Why is it becoming harder for you? Let me know how I can be of assistance to help you further contribute.

@barakman
Copy link
Contributor Author

barakman commented Aug 8, 2018

@nventuro:
Harder for me to merge your code into my repo, only as long as I apply all these fixes locally.
Once I can get you to apply them on your side (assuming those fixes are correct), then it becomes much easier for me (i.e., more or less a pure copy-paste).

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.

2 participants