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

feat: solhint + solhint-plugin-prettier for linting #19

Merged
merged 4 commits into from
Sep 27, 2021

Conversation

gg2001
Copy link
Contributor

@gg2001 gg2001 commented Sep 24, 2021

Im not sure if adding solhint is the best idea since it makes the node_modules folder more bloated, but I find it very useful. Would like to hear thoughts on adding this.

@transmissions11
Copy link
Contributor

Wouldn’t this block using lint if the project violates a solhint rule? I think you need to use --fix in the solhint script to fix

@transmissions11
Copy link
Contributor

this reminded me that we should probably have a separate lint-check and lint-fix pair of scripts to allow both fixing in development and checking for conformity in CI

@gg2001
Copy link
Contributor Author

gg2001 commented Sep 24, 2021

Makes sense, I just added --fix to the solhint script

I also added a yarn lint:check for just doing a check
The yarn lint by default does a fix

@transmissions11
Copy link
Contributor

transmissions11 commented Sep 24, 2021

Nice! I think you can simplify the write/fix scripts by just having them do like yarn solhint:check -- --fix

@gg2001
Copy link
Contributor Author

gg2001 commented Sep 25, 2021

Thanks! Looks like the -- isn't necessary anymore so I just put it as yarn solhint:check --fix

Screen Shot 2021-09-25 at 2 35 16 pm

@transmissions11
Copy link
Contributor

ah nice sorry im used to npm not yarn

@transmissions11
Copy link
Contributor

can you simplify the prettier fix script too? otherwise lgtm

@gg2001
Copy link
Contributor Author

gg2001 commented Sep 27, 2021

Done!

@gakonst gakonst merged commit 0b81a54 into foundry-rs:master Sep 27, 2021
@gakonst
Copy link
Member

gakonst commented Sep 27, 2021

LGTM. Thank you @gg2001 for the contribution and @transmissions11 for the PR reviews. Really appreciate it.

sambacha pushed a commit to sambacha/dapptools-template that referenced this pull request Oct 9, 2021
* feat: solhint

* fix: add --fix to solhint script + add a lint check script

* chore: simplify solhint script

* chore: simplify prettier script
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

3 participants