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

Switch to ESLint from JSHint+JSCS #1534

Merged
merged 1 commit into from
Jul 19, 2016
Merged

Conversation

markelog
Copy link
Contributor

Fixes #1533

var errorcount,

// The last specified tasks message.
lastInfo;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@markelog markelog Jul 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

"grunt-contrib-nodeunit": "~0.4.1",
"grunt-contrib-watch": "~1.0.0",
"grunt-jscs": "~2.8.0",
"grunt-eslint": "^18.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we can do that!

@vladikoff
Copy link
Member

@markelog awesome, thanks so much!

@markelog
Copy link
Contributor Author

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

@vladikoff
Copy link
Member

👍

@markelog
Copy link
Contributor Author

Oh, you already merged :)

@vladikoff
Copy link
Member

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

@markelog
Copy link
Contributor Author

Ok, cool

@markelog
Copy link
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants