Add linter and travis.yml #97

2019-04-12T22:11:26Z

fsmanuel commented

2019-04-12 22:11:26 +00:00

(Migrated from github.com)

Let's discuss the linting rules!
I adopted Bastis use of a space between the function name and the parentheses:
serialize () {
and added a comma-dangle rule.

I have no idea on the solidity rules. Right now it complains about spaces...
https://github.com/protofire/solhint
https://github.com/protofire/solhint/blob/master/docs/rules.md

Let's discuss the linting rules! I adopted Bastis use of a space between the function name and the parentheses: `serialize () {` and added a `comma-dangle` rule. I have no idea on the solidity rules. Right now it complains about spaces... https://github.com/protofire/solhint https://github.com/protofire/solhint/blob/master/docs/rules.md

👍 1

fsmanuel commented

2019-04-12 23:18:21 +00:00

(Migrated from github.com)

I decided to use the solhint:recommended ruleset.

I decided to use the `solhint:recommended` ruleset.

bumi commented

2019-04-13 07:10:41 +00:00

(Migrated from github.com)

yay! 👍

I will look at that solidity code and try to update it. - no idea about the rules there.

yay! :+1: I will look at that solidity code and try to update it. - no idea about the rules there.

raucao (Migrated from github.com) approved these changes 2019-04-13 11:31:43 +00:00

raucao (Migrated from github.com) left a comment

That dangling comma looks slightly revolting to me, but I'll give it a try if you think it's better.

Definitely great to have linting in the first place!

(By the way, Travis is failing.)

That dangling comma looks slightly revolting to me, but I'll give it a try if you think it's better. Definitely great to have linting in the first place! (By the way, Travis is failing.)

fsmanuel commented

2019-04-13 13:08:24 +00:00

(Migrated from github.com)

I like the dangling comma because of the cleaner diff.

Here are the JS errors:

/home/travis/build/67P/kredits-contracts/lib/kredits.js
  54:11  error  Unexpected console statement  no-console
  65:5   error  Unexpected console statement  no-console
  83:5   error  Unexpected console statement  no-console

/home/travis/build/67P/kredits-contracts/lib/utils/validator.js
  10:94  error  Unnecessary escape character: \+  no-useless-escape
  10:97  error  Unnecessary escape character: \-  no-useless-escape

What should we do about the no-console rules? Should we allow it globally or add a log() utils function and allow it there?

I like the dangling comma because of the cleaner diff. Here are the JS errors: ``` /home/travis/build/67P/kredits-contracts/lib/kredits.js 54:11 error Unexpected console statement no-console 65:5 error Unexpected console statement no-console 83:5 error Unexpected console statement no-console /home/travis/build/67P/kredits-contracts/lib/utils/validator.js 10:94 error Unnecessary escape character: \+ no-useless-escape 10:97 error Unnecessary escape character: \- no-useless-escape ``` What should we do about the `no-console` rules? Should we allow it globally or add a `log()` utils function and allow it there?

fsmanuel commented

2019-04-15 11:54:32 +00:00

(Migrated from github.com)

I found another linter https://github.com/duaraghav8/Ethlint it is used by aragon-apps so maybe we should consider to use it as well...

bumi commented

2019-04-24 18:00:23 +00:00

(Migrated from github.com)

I did not have the time to look at the solidity stuff and also probably won't in the next days.
maybe we merge this without the solidity part?

I did not have the time to look at the solidity stuff and also probably won't in the next days. maybe we merge this without the solidity part?

fsmanuel commented

2019-04-24 18:11:10 +00:00

(Migrated from github.com)