CI setup and minor enhancements #2
Conversation
There was a problem hiding this comment.
We could also update the tests to use async/await notation which imo makes it easier to read (https://github.com/parity-contracts/auto-updater/blob/andre-fix-more-lints/test/simple_operations.js).
contracts/PrivateContract.sol
Outdated
| // and allows its change only if all the Validators (from a static list, initialized in constructor) | ||
| // have signed a new state (hashed together with a current nonce, for replay protection). | ||
| // | ||
| // |
There was a problem hiding this comment.
I would put the license header before the previous block comment describing the contract.
.travis.yml
Outdated
| script: | ||
| - yarn run ci | ||
| after_script: | ||
| - cat coverage/lcov.info | yarn run coveralls |
package.json
Outdated
| }, | ||
|
|
||
| "devDependencies": { | ||
| "babel-polyfill": "^6.26.0", |
There was a problem hiding this comment.
I didn't add babel in auto-updater because the js code will only be run on node, assuming a recent version of node it has all the features we need.
contracts/PrivateContract.sol
Outdated
| nonce = 1; | ||
| } | ||
|
|
||
| function getValidators() public constant returns (address[]) { |
There was a problem hiding this comment.
I would break the modifier list. I saw this code style in some other repo and I think it makes it easier to notice the visibility of a function. But I'm open to change if people don't like it.
There was a problem hiding this comment.
Do we have some accepted coding style guide for solidity contracts? As a result this contract looks like rust code )
There was a problem hiding this comment.
No I don't think we have any defined style guide but I think it would be good if we could have some consistency across our different contracts. I followed the style guide here https://solidity.readthedocs.io/en/v0.4.21/style-guide.html, some of that stuff is actually reported by the linter. And I added the rule of always breaking modifiers into new lines. Let me know if you think we should use a different style or add/remove some rules.
There was a problem hiding this comment.
Thanks for the link. It's very helpful (I'm not the experienced solidity coder). So it's important to have such guide fixed and not a big deal what's inside. I can bear anything :-) As for breaking modifiers both options are ok.
|
@andresilva Weirdly enough, tests didn't work for me without explicit inclusion of |
|
@kirushik What |
|
@tomusdrw Well, that was the first thing I've checked for: I think it might be |
No description provided.