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.

Sign up for GitHub

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

Jump to bottom

Try to get rid of compiling warnings #385

Closed
bingen opened this issue Aug 19, 2018 · 3 comments · Fixed by #383
Closed

Try to get rid of compiling warnings #385

bingen opened this issue Aug 19, 2018 · 3 comments · Fixed by #383
Assignees
@bingen
Labels
enhancement

Comments

@bingen
Copy link
Contributor

bingen commented Aug 19, 2018

Main rationale:
ethereum/solidity#4315 (comment)
Besides, it would make easier to spot compiling errors while developing, that sometimes get buried among long list of warnings.
@bingen bingen self-assigned this Aug 19, 2018
@izqui
Copy link
Contributor

izqui commented Aug 20, 2018

#383 removes almost all the compiler warnings in our code. I actually listed the warnings that are still standing.

@bingen
Copy link
Contributor Author

bingen commented Aug 20, 2018

So should we close this one? Do you want to put a "Closing .." footer to #383 so this one gets closed once that one is merged?

@izqui
Copy link
Contributor

izqui commented Aug 20, 2018

Yep, we should close this with #383 and then create issues for the remaining warnings so we can track them separately.

@izqui izqui mentioned this issue Aug 20, 2018
Merged
6 tasks
sohkai pushed a commit that referenced this issue Aug 26, 2018
@izqui @sohkai
Update to solc 0.4.24 (#383)
bfed147 
There's a ton of changes in this PR, the main goals were updating to the new compiler version and doing some clean up to reduce the number of compiler warnings (close #385):

- Bumps truffle dependency to a more recent one that comes with Solidity 0.4.24.
- Fixes almost all compiler warnings we care about
- Changes all constructors to the new `constructor()` syntax
- Adds `emit` keyword to all events
- Passing anything other than a bytes array to `keccak256()` is now deprecated, a manual `abi.encodePacked(...)` has been added when calculating all hashes.
- 💥Removes all the fun [custom abi encoding](https://github.com/aragon/aragonOS/pull/383/files#diff-6d2e88efa27690b63879e31ffb310725L9) in favor of using `abi.encodeWithSelector` 
- Update solidity-coverage to `0.5.7`. Solidity mocks and tests had to be moved to `contracts/tests` so they could be instrumented correctly (see sc-forks/solidity-coverage#182) and not interfere with the ABI-swapping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bingen bingen

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update to solc 0.4.24 aragon/aragonOS
2 participants
@izqui @bingen