Add ESLint #990
Add ESLint #990
Conversation
|
@mleanos fixed Gulp test, it was actually an issue with me leaving out |
| @@ -101,6 +101,10 @@ module.exports = function (grunt) { | |||
| } | |||
| } | |||
| }, | |||
| eslint: { | |||
| options: {}, | |||
There was a problem hiding this comment.
if there are no options that we're passing can we just not include it?
There was a problem hiding this comment.
@lirantal we may want to add options once we finalize formatting, so I figured I would just leave it.
There was a problem hiding this comment.
If it's redundant we can remove it until we add something there.
|
@ilanbiala what formatting does this eslint config compatible with? is that for the current project formatting until we decide on a final one? |
|
@lirantal it's pretty close to the formatting we have right now. Obviously we can add any other specific stuff once we discuss it in the code formatting issue. This PR is meant more for just adding in support and we'll tweak all the rules later in a discussion. |
|
@lirantal let me know if I can merge, I'll hold off just to make sure. |
|
LGTM! |
|
Tested, and LGTM. I'm not completely clear on these eslint formatting settings, but based off of the fact that the only formatting changes that were required in the other PR were minor spacing issues, then I think it is line with our current coding styles. Once we decide on any changes to our styles, we can adjust these settings. But for now, this is probably good to merge to get our initial ESLint support out. |
|
@ilanbiala Let's go for it. It's better to have something, than nothing at all or wait for the final coding convention to finalize, which god only knows when will it happen. |
Fixes #763.
@codydaig @mleanos @lirantal made a new PR so I didn't have to work off master on my fork.