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

Solium run and Solium version update #556

Closed
wants to merge 4 commits into from

Conversation

azavalla
Copy link
Contributor

@azavalla azavalla commented Nov 12, 2017

refs #75 and #555
Fixes #656

Solium config:

  • "custom-rules-filename" is deprecated.

  • "no-with" rule is also deprecated.

  • "double-quotes" rule is deprecated. It was replaced by ["quotes", "double"].

  • Added "mixedcase" (is included in solium:all). As pointed out on Linter: upgrade to v1 #555, bugs with this rule where fixed. Is there another reason to not to use it?

  • Added security plugin. Set severity for security rules appropriately.
    https://github.com/duaraghav8/solium-plugin-security#list-of-rules

  • These rules are included in solium:all, so they where removed:
    imports-on-top
    variable-declarations
    array-declarations
    operator-whitespace
    lbrace
    camelcase
    uppercase
    no-empty-blocks
    no-unused-vars
    blank-lines
    indentation
    whitespace
    deprecated-suicide
    pragma-on-top

Other:
Refactored contracts to comply with the new rules.
Abstract base contracts constructors marked internal instead of public.

Issues:
"quotes" rule doesn't seem to be working.

@duaraghav8
Copy link

Thanks for making the PR @azavalla!

(Just for general information)
CI builds can fail because of Solium. This happens when you specify a particular lint rule as error and that rule flags some code. A solium error always results in exit code > 0. You could set the rule to warning to avoid build fails but still be notified of the lint errors.

@azavalla
Copy link
Contributor Author

Thanks for the heads-up @duaraghav8! Any ideas on the "quotes" rule issue?

@duaraghav8
Copy link

@azavalla I don't follow. Can you point me to which quotes-related issue you're talking about?

@azavalla
Copy link
Contributor Author

azavalla commented Nov 13, 2017

@duaraghav8 there are both single and double quoted string literals, but Solium doesn't seem to catch them.
EDIT: I see Solium throws the error when they are declared as strings, but not otherwise.

@duaraghav8
Copy link

duaraghav8 commented Nov 14, 2017

ah yes that's a limitation of the lint rule currently, sorry about that. We'll be fixing it in a subsequent release but as of now it only flags strings.

UPDATE: It's fixed!

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.

Merge solium PR and integrate into CI/testing
3 participants