Build: Switch from JSHint/JSCS to ESLint (+ other housekeeping) #238
Conversation
We can't format package.json as all other JSON/JS files since it prevents us from using npm commands updating package.json - they don't preserve the formatting. jQuery Core has switched long time ago. This commit doesn't introduce any changes to package.json except reformatting it to follow the format produced by npm.
The previous way required listing all the loaded packages individually which was sub-optimal. jQuery Core also uses this package.
|
@mgol, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dmethvin, @markelog and @gibson042 to be potential reviewers. |
| @@ -1,80 +1,81 @@ | |||
| { | |||
| "name": "jquery-migrate", | |||
There was a problem hiding this comment.
You probably should change
Line 13 in 5acafbc
There was a problem hiding this comment.
Why? It already says "2 spaces" so this file was incorrectly formatted...
There was a problem hiding this comment.
O, can't really see tabs vs spaces thingy in github UI
|
|
||
| // Integrate jQuery migrate specific tasks | ||
| grunt.loadTasks( "build/tasks" ); | ||
|
|
||
| // Just an alias | ||
| grunt.registerTask( "test", [ "qunit" ] ); | ||
|
|
||
| grunt.registerTask( "lint", [ "jshint", "jscs" ] ); | ||
| grunt.registerTask( "lint", [ "eslint:dev", "eslint:dist" ] ); |
There was a problem hiding this comment.
Why two tasks? Why not just eslint?
There was a problem hiding this comment.
This is one task with 2 targets. Not specifying the targets manually would first invoke the dist one and then quit; that makes it way harder to know what to fix.
There was a problem hiding this comment.
Not specifying the targets manually would first invoke the dist one and then quit
How this is an issue?
that makes it way harder to know what to fix
Didn't it prints path to problematic file?
| @@ -1,11 +1,14 @@ | |||
| /* exported migrateWarn, migrateWarnFunc, migrateWarnProp */ | |||
There was a problem hiding this comment.
|
The problematic file in question would always be dist/jquery-migrate.js.
Hardly a good experience! I want this file to be pointed at only if we
already verified that source files pass the linter.
|
|
@mgol Make sense |
|
Don't be so defensive btw :) |
|
I wasn't trying to be defensive, sorry if it sounded that way. :) |
Fixes #236
It makes sense to review it commit by commit, the changes make sense to land separately.