search

adobe-photoshop / spaces-design

Adobe Photoshop Design Space
http://adobe-photoshop.github.io/
Other
852 stars 74 forks source link

Replace jshint with eslint #3640

Closed iwehrman closed 8 years ago

iwehrman commented 8 years ago

This PR replaces our jshint grunt task with an improved eslint grunt task. eslint is better for three reasons:

  1. it has better support for ES2015 syntax
  2. it works better than jshint, finding many errors that jshint should have caught but didn't
  3. it works better with SublimeLinter, so it's now actually possible to do real-time linting in ST3.

As a result of 2) above, many indentation errors had to be fixed. eslint also caught a number of instances of unused variables, some of which were genuine bugs. (Related to that, see #3639.)

To make the linters work correctly with ST3, use Package Control to install Babel, SublimeLinter, SublimeLinter-jscs and SublimeLinter-contrib-eslint. Next, globally install jscs and eslint globally with npm install -g jscs eslint. You should verify that the globally installed versions are approximately the same as the dependencies of our grunt tasks. (I.e., if you had an old version installed globally, npm install won't necessarily update it.)

shaoshing commented 8 years ago

@iwehrman thanks for upgrading and fixing all the issues. eslint looks pretty sweet! :+1: Before merging, I noticed some new idention problems due to the changes, so back to you.

iwehrman commented 8 years ago

@shaoshing @volfied: thanks for taking a look. In some cases I agree that the indentation is incorrect but in most cases I think eslint is putting things in the "correct" place. In particular, I think this is "correct":

var foo = new Promise(function (resolve) {
    resolve();
})
    .bind(this);

At least, I think this formatting is a valid consequence of the four-space indentation rule that eslint is enforcing. At first I didn't like it, but now I think it's actually reasonable. In any case, I'm not sure how I would explain to eslint that it should be any other way. (JSCS is agnostic about this; it accepts either indentation.)

I'll look into fixing the other issues, but in the meantime, what's your reaction to that?

iwehrman commented 8 years ago

(Also, I agree with the @shaoshing's suggested formatting workaround when it's possible.)

iwehrman commented 8 years ago

OK, I've used @shaoshing's suggested workaround for all the oddities he pointed out. Back to @shaoshing!

shaoshing commented 8 years ago

Superb!

iwehrman commented 8 years ago

But don't forget adobe-photoshop/spaces-adapter#183 :)