Switch to ESLint from JSHint+JSCS #1534

Merged
merged 1 commit into from Jul 19, 2016

Projects

None yet

4 participants

@markelog
Contributor

Fixes #1533

@markelog markelog and 1 other commented on an outdated diff Jul 15, 2016
lib/grunt/task.js
@@ -1,5 +1,11 @@
'use strict';
+// Keep track of the number of log.error() calls.
+var errorcount,
+
+ // The last specified tasks message.
+ lastInfo;
@markelog
markelog Jul 15, 2016 Contributor

This is a difference between JSCS and ESLint one-var/disallowMultipleVarDecl rules, i figure it is easier to change the code then to propose changes in ESLint

@shama
shama Jul 15, 2016 Member

Are you saying ESLint doesn't allow multiple var statements?

@cowboy has some good opinions on that. So Grunt would definitely prefer to use multiple var statements.

@markelog
markelog Jul 15, 2016 edited Contributor

With

"one-var": [
  "error",
  {
    "uninitialized": "always",
    "initialized": "never"
  }
]

It doesn't allow "initialized" values with different var keywords. I believe it satisfy the idea in the blog post you mentioned (see "In summary" at the end) -

My rule is as-follows: I’ll put multiple comma-separated declarations in a single var statement, but only if they don’t have assignments.

// SIMPLE AND SEXY

var foo, bar, baz;
var abc = 1;
var def = 2;

In any case, if you want me to change this rule or omit it, I can do that too

@shama
shama Jul 16, 2016 Member

Ah cool. Thanks for pointing that out. Could we put the declarations onto one line and just concat the code comments with and?

@markelog
markelog Jul 19, 2016 Contributor

done

@markelog markelog commented on the diff Jul 15, 2016
package.json
"grunt-contrib-nodeunit": "~0.4.1",
"grunt-contrib-watch": "~1.0.0",
- "grunt-jscs": "~2.8.0",
+ "grunt-eslint": "^18.0.0",
@markelog
markelog Jul 15, 2016 Contributor

19.0.0 is not used since it doesn't supports node < 4, which means you can't use ESLint 3.0, we can apply same approach as in jquery repo though - jquery/jquery@030191a

@markelog
markelog Jul 15, 2016 Contributor

But if you do, i would suggest to do this in separate commit

@vladikoff
vladikoff Jul 19, 2016 Member

yeah we can do that!

@markelog markelog Switch to ESLint from JSHint+JSCS
Fixes #1533
4932ed5
@vladikoff vladikoff merged commit f152402 into gruntjs:master Jul 19, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
jQuery Foundation CLA All authors have signed the CLA
Details
@vladikoff
Member

@markelog awesome, thanks so much!

@markelog
Contributor

@vladikoff wait a bit plz, one more change coming up

@vladikoff
Member

👍

@markelog
Contributor

Oh, you already merged :)

@vladikoff
Member

@markelog yea you can send another PR, no worriES!

@markelog
Contributor

Ok, cool

@markelog
Contributor

Never mind, new changes are blocked by the eslint/eslint#3458

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment