Skip to content
This repository was archived by the owner on Jul 31, 2019. It is now read-only.

Conversation

@badrmodoukh
Copy link
Contributor

@badrmodoukh badrmodoukh commented Apr 9, 2017

@Pomax I installed eslint for grunt and modified the Gruntfile.js file to include the eslint task.
The only rule I can think of to enable and check for is the indent rule. I added it in the .eslintrc.json file and specified that the indent needs to be 2 spaces. If there are any other rules I need to add please let me know.
I also specified the ecmaVersion for the parser to be 6 because if I don't it will give many parsing errors like the keyword let is reserved and const is reserved.
Currently when I run npm test I get 7 errors because some files don't use 2 spaces. This is how the errors look:
screen shot 2017-04-09 at 2 59 35 pm

@badrmodoukh
Copy link
Contributor Author

I fixed the above errors

@humphd humphd self-requested a review April 13, 2017 02:22
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

I think this looks good. Any objections? We can extend the rules as we go.

@humphd
Copy link
Contributor

humphd commented Apr 13, 2017

cc @dsych. Let's build on this PR to also add prettier fix like we are doing in Brackets. mozilla/brackets#703

@Pomax Pomax self-assigned this Apr 13, 2017
@flukeout
Copy link
Contributor

flukeout commented Jun 2, 2017

Can we merge this @humphd - seems like the problems were addressed.

@humphd
Copy link
Contributor

humphd commented Jun 16, 2017

Tested this again today, and it needs a few things:

  • This needs to get rebased onto Master, where I found new errors
  • The jshint stuff needs to get removed, since eslint does a better job
  • We need a set of rules
  • We need to fix the errors it produces.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

See my earlier comment.

@gideonthomas
Copy link
Contributor

Since we're switching to webpack, we would need to run eslint there. Feel free to take on that issue once the webpack code is merged in. Closing, in favor of that.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants