Skip to content
This repository has been archived by the owner on Jan 14, 2020. It is now read-only.

Solium (solidity linter) #523

Merged
merged 20 commits into from
Sep 21, 2018
Merged

Solium (solidity linter) #523

merged 20 commits into from
Sep 21, 2018

Conversation

tyleryasaka
Copy link
Contributor

Checklist:

  • Code contains relevant tests for the problem you are solving
  • Ensure all new and existing tests pass
  • Update any relevant READMEs and docs

Description:

  • Integrates solium
  • Runs solium with npm test
  • Addresses current errrors/warnings raised by solium
  • Creates npm run format:solidity command

Makes progress towards #513
Resolves #501

gaboesquivel and others added 19 commits September 20, 2018 14:40
@gaboesquivel @tyleryasaka
add lint npm script ( solium )
1c7728a 
rel #167
@tyleryasaka
Update solium command
72e917e
@tyleryasaka
Change solium config to match the official Solidity style guide
0c38640 
https://solidity.readthedocs.io/en/v0.4.24/style-guide.html
@tyleryasaka
Specify correct directory to lint
dc483a8
@tyleryasaka
Format with npm run format:solidity
b55a702
@tyleryasaka
Fix solidity indentation
ac62897
@tyleryasaka
Fix solium error "consequent..."
b845611 
"Consequent should exist exactly on the line after condition."
@tyleryasaka
Change to solium:recommended
e0e5f0b
@tyleryasaka
Add .soliumignore
bea1511
@tyleryasaka
Turn off 2 security rules
a5e52a5 
This should certainly be done with caution and I don't feel good about it. However, we've made intentional decisions in both of these cases to ignore these rules. We can't comply with these rules without making major design changes.
@tyleryasaka
Turn off max-len warning
c4dcaaa
@tyleryasaka
Fix whitespace warnings
d18597d
@tyleryasaka
Turn off no-empty-blocks
be50d6f
@tyleryasaka
Fix path to migrations.sol in soliumignore
63fd703
@tyleryasaka
Turn no-block-members warning off
d287492 
We are blatantly ignoring this rule right now
@tyleryasaka
Turn off no-low-level-calls warnings
6b4070d 
We are only violating this rule once; if we decide we want to follow this rule in the future, we can do that.
@tyleryasaka
Fix solium indentation error
ad60635
@tyleryasaka
Run solium as part of npm test command
405a0d9
@tyleryasaka
Disable solium no-send warning
7d7cddb
@tyleryasaka tyleryasaka requested review from nick and cuongdo September 20, 2018 22:38
@tyleryasaka tyleryasaka mentioned this pull request Sep 20, 2018
Closed
nick
nick approved these changes Sep 20, 2018
View reviewed changes
Copy link
Contributor

@nick nick left a comment

Choose a reason for hiding this comment

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

Nice 👍

cuongdo
cuongdo approved these changes Sep 21, 2018
View reviewed changes
Copy link
Contributor

@cuongdo cuongdo left a comment

Choose a reason for hiding this comment

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

Love it! One of my pet peeves was the 2-space indentation, which is annoying when using Remix.

@cuongdo cuongdo merged commit 4b15f4c into master Sep 21, 2018
@cuongdo cuongdo deleted the tyleryasaka/solium branch September 21, 2018 01:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cuongdo cuongdo cuongdo approved these changes

@nick nick nick approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tyleryasaka @nick @cuongdo @gaboesquivel