-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Use the Chart.js shared ESLint config #5112
Conversation
Well, shareable config doesn't seem supported by CodeClimate (41 fixed issues) |
Looks like the 41 issues were all either statement count or the complexity. Is it OK to lose those? |
I think it actually lints nothing, as if the |
Lots of discussion in codeclimate/codeclimate-eslint#86 that may be relevant |
Linting will still run for builds & PRs as its part of the CI as well. https://travis-ci.org/chartjs/Chart.js/builds/325810760#L2166 |
I was reading it but no real solution. The closest is the |
Maybe there's a codeclimate alternative we could consider? If the cost is low we could self host something, but I'd prefer not to do that if possible |
709c05e
to
64cc687
Compare
@etimberg I agree with you, we already run ESLint against our code during the build process on Travis and if there are errors, we don't really care about CC result. I think we still want complexity and statement count to be reported by CC because it highlights part of the code that need to be refactored. One solution would be to create a custom
rules:
complexity: [2, 10]
max-statements: [2, 30]
prepare:
fetch:
- url: 'https://raw.githubusercontent.com/chartjs/eslint-config-chartjs/master/.eslintrc.cc.yml'
path: '.eslintrc.yml'
engines:
# ... |
64cc687
to
f53728e
Compare
It seems to work but actually, CC already have those checks enabled: https://docs.codeclimate.com/v1.0/docs/advanced-configuration#section-default-checks. So maybe the cleanest/simplest is to disable the ESLint CC plugin, what do you think? |
That works for me |
An ESLint shareable config has been created (from this repository) in the attempt to homogenize Chart.js hosted projects and plugins style. Rename `.eslintrc` files to `.eslintrc.yml` since the name has been deprecated. Fix the CC badge (maintainability) and disable CodeClimate ESLint plugin because it doesn't support custom shareable config but also because it already executes relevant checks as part of the regular process.
bbd8cac
to
bf2dd32
Compare
Commits squashed (CC ESLint disabled), I also fixed the CC badge |
Looks good to me. Can I merge? |
Yes, it needs at least one approval |
Will do tonight. Can’t on mobile |
An ESLint shareable config has been created (from this repository) in the attempt to homogenize Chart.js hosted projects and plugins style. Rename `.eslintrc` files to `.eslintrc.yml` since the name has been deprecated. Fix the CC badge (maintainability) and disable CodeClimate ESLint plugin because it doesn't support custom shareable config but also because it already executes relevant checks as part of the regular process.
An ESLint shareable config has been created (from this repository) in the attempt to homogenize Chart.js hosted projects and plugins style. Rename
.eslintrc
files to.eslintrc.yml
since the ([name has been deprecated(https://eslint.org/docs/user-guide/configuring#configuration-file-formats)) to.eslintrc.yml
..Fix the CC badge (maintainability) and disable CodeClimate ESLint plugin because it doesn't support custom shareable config but also because it already executes relevant checks as part of the regular process.