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

Upgrade to Solidity v0.5.0 #21

Merged
merged 5 commits into from
Feb 26, 2019
Merged

Upgrade to Solidity v0.5.0 #21

merged 5 commits into from
Feb 26, 2019

Conversation

fubuloubu
Copy link
Contributor

  • Style changes
  • Added memory everywhere to bytes and strings
  • target.call(data) now returns a tuple

GNSPS and others added 3 commits February 11, 2019 20:00
* Vanity updates

* Add type cast method to `bytes32`

* Update to the natSpec comment on top of both libraries 😄

Though I really like Nick Johnson's string library this has been built entirely by me (as opposed to what the NatSpec comment in the top said since the last commit 😄) I just copied the comment layout on top of his library and forgot to change the author 😂

* Adding Solidity highlight to gitattribute

🎉 😄

* Update package versions & prepare for NPM publish

* Create a NPM ignore file to keep `truffle.js` from being published

* Syntax & pragmas updates for v0.5.0

* Update README to reflect NPM publish

* Another README update

* Support for typecasting into uint8, uint16, uint32 (GNSPS#19)

* Add uint8, uint16, uint32

* Unit test for uint8, uint16, uint32

* Removing Truffle as a dependency

* Bump version to v0.0.6
* Style changes
* Added `memory` everywhere to bytes and strings
* `target.call(data)` now returns a tuple
@GNSPS GNSPS changed the base branch from master to develop February 25, 2019 17:14
@GNSPS
Copy link
Owner

GNSPS commented Feb 26, 2019

I just changed the target branch to develop to follow the same flow for all the previous versioned releases (merged the file in the latest commit) and updated the README with a note about compiler support.
Also made the change to have the CI install Truffle v5 and use solc v0.5.1 but tests ran locally seem to be failing for me for some reason. Do they pass if you run $ truffle test, @fubuloubu ?

ThrowProxy was giving problems while testing with the JS-VM with the Constantinople hard-fork. So I changed the tests to stop using that pattern and use function selectors since that's now a thing.

It is cleaner now.
@GNSPS
Copy link
Owner

GNSPS commented Feb 26, 2019

Ok, test are passing now on my machine. Overall ditching the ThrowProxy was probably a good v0.5.x improvement either way, so... 😄

Let me know if you're ok with me merging the request as is @fubuloubu .

@fubuloubu
Copy link
Contributor Author

Looking good for me. The tests complain about the following unused variables:

Compilation warnings encountered:                                                                                                        
                                                                                                                                         
/home/bryant/Documents/solidity-bytes-utils/test/TestBytesLib1.sol:262:9: Warning: Unused local variable.                                
        bytes memory postBytes4 = hex"f00dfeed";                                                                                         
        ^---------------------^                                                                                                          
,/home/bryant/Documents/solidity-bytes-utils/test/TestBytesLib1.sol:263:9: Warning: Unused local variable.                               
        bytes memory postBytes31 = hex"f00d000000000000000000000000000000000000000000000000000000feed";                                  
        ^----------------------^                                                                                                         
,/home/bryant/Documents/solidity-bytes-utils/test/TestBytesLib1.sol:264:9: Warning: Unused local variable.                               
        bytes memory postBytes32 = hex"f00d00000000000000000000000000000000000000000000000000000000feed";                                
        ^----------------------^    

It looks like in your tests, this doesn't match up with what I think is intended (I'd expect each preBytes to do something with it's corresponding postBytes), but that's outside the scope of this PR.

@GNSPS GNSPS merged commit d942a00 into GNSPS:develop Feb 26, 2019
@fubuloubu fubuloubu deleted the solidity-v0.5 branch February 26, 2019 19:46
@fubuloubu fubuloubu changed the title (WIP) Upgrade to Solidity v0.5.0 Upgrade to Solidity v0.5.0 Feb 26, 2019
GNSPS added a commit that referenced this pull request Feb 27, 2019
* (WIP) Upgrade to Solidity v0.5.0 (#21)

* (refactor) Changed codebase to Solidity v0.5.0

* Style changes
* Added `memory` everywhere to bytes and strings
* `target.call(data)` now returns a tuple

* Adding note about compiler support in the README + typo fix

* Replaced the ThrowProxy pattern for a better v0.5.x one

ThrowProxy was giving problems while testing with the JS-VM with the Constantinople hard-fork. So I changed the tests to stop using that pattern and use function selectors since that's now a thing.

It is cleaner now.

* Update package-lock.json

* Bump version to v0.0.7

* Bump version in package-lock.json

* Update dependency versions

* Fix typo in package-lock.json
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.

None yet

2 participants