ESLint and cleanup #3256
Merged
ESLint and cleanup #3256
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change the linter in gulp tasks to be consistent with Code Climate results which are based on ESLint using .eslintrc options. However, defaults Code Climate rules are too strict, so turn as warnings the 'complexity' and 'max-statements' rules (other errors has been fixed). Note that the Gulp task name has been changed for `gulp lint`.
indent: [error, tab] (http://eslint.org/docs/rules/indent)
curly: [error, all] (http://eslint.org/docs/rules/curly)
simonbrunel
force-pushed
the
code-cleanup
branch
from
054904f to
7c93255
Compare
|
Looks good to me. I think that we should add even more style standards going forward to make sure everything has the same styling. |
|
Agree, I enforced the recurring ones in submitted PR but if you think about some others? |
| ]; | ||
|
|
||
| gulp.task('bower', bowerTask); | ||
| gulp.task('build', buildTask); | ||
| gulp.task('package', packageTask); | ||
| gulp.task('coverage', coverageTask); | ||
| gulp.task('watch', watchTask); | ||
| gulp.task('jshint', jshintTask); | ||
| gulp.task('test', ['jshint', 'validHTML', 'unittest']); | ||
| gulp.task('lint', lintTask); |
There was a problem hiding this comment.
I thought we were also changing the test tasks and gulp unittestWatch was going way in favour of gulp test -watch. Or will that be a separate PR?
There was a problem hiding this comment.
I'm still unsure how to implemented it the best way, so will be another PR.
|
Added a few comments. Looks great @simonbrunel |
simonbrunel
added a commit
to simonbrunel/Chart.js
that referenced
this pull request
Mar 25, 2017
Coverage data are now generated by running `gulp unittest` with the `--coverage` argument: unit tests are now executed a single time on Travis. The gulp `coverage` task has been removed and `karma.coverage.conf.ci.js` merged into `karma.conf.ci.js`. Update documentation with gulp commands (and remove them from `README.md`) and remove unused `config.jshintrc` (oversight from chartjs#3256). Delete `thankyou.md` which has been merged into `README.md`.
simonbrunel
added a commit
that referenced
this pull request
Mar 25, 2017
Coverage data are now generated by running `gulp unittest` with the `--coverage` argument: unit tests are then executed a single time on Travis. The gulp `coverage` task has been removed and `karma.coverage.conf.ci.js` merged into `karma.conf.ci.js`. Update documentation with gulp commands (and remove them from `README.md`) and remove unused `config.jshintrc` (oversight from #3256). Delete `thankyou.md` which has been merged into `README.md`.
ericnkatz
pushed a commit
to ericnkatz/Chart.js
that referenced
this pull request
May 24, 2017
Coverage data are now generated by running `gulp unittest` with the `--coverage` argument: unit tests are then executed a single time on Travis. The gulp `coverage` task has been removed and `karma.coverage.conf.ci.js` merged into `karma.conf.ci.js`. Update documentation with gulp commands (and remove them from `README.md`) and remove unused `config.jshintrc` (oversight from chartjs#3256). Delete `thankyou.md` which has been merged into `README.md`.
roicos
pushed a commit
to roicos/Chart.js
that referenced
this pull request
Aug 21, 2017
Coverage data are now generated by running `gulp unittest` with the `--coverage` argument: unit tests are then executed a single time on Travis. The gulp `coverage` task has been removed and `karma.coverage.conf.ci.js` merged into `karma.conf.ci.js`. Update documentation with gulp commands (and remove them from `README.md`) and remove unused `config.jshintrc` (oversight from chartjs#3256). Delete `thankyou.md` which has been merged into `README.md`.
exwm
pushed a commit
to exwm/Chart.js
that referenced
this pull request
Apr 30, 2021
ESLint and cleanup
exwm
pushed a commit
to exwm/Chart.js
that referenced
this pull request
Apr 30, 2021
Coverage data are now generated by running `gulp unittest` with the `--coverage` argument: unit tests are then executed a single time on Travis. The gulp `coverage` task has been removed and `karma.coverage.conf.ci.js` merged into `karma.conf.ci.js`. Update documentation with gulp commands (and remove them from `README.md`) and remove unused `config.jshintrc` (oversight from chartjs#3256). Delete `thankyou.md` which has been merged into `README.md`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove unused npm packages, use ESLint instead of JSHint in Gulp to be consistent with Code Climate results and enforce indent and curly rules.
Many methods are too complex, it would require an important effort to rewrite them but I'm sure it will be only benefits since we will be able to refactor a lot. For now, I simply make these rules more permissive for the gulp lint task (not for Code Climate).