A new version of JSHint is out: http://www.jshint.com/blog/2013-03-05/1-1-0/ and http://www.jshint.com/blog/2013-05-07/2-0-0/ it introduces a few nice options.

I'd like to add the following:

  1. "unused": "vars": This would show something fishy going on in the code, uneeded parameters are too many in core and don't highlight potential problems the same way unused vars do.
  2. "strict": true: We're using strict mode everywhere, this can enforce it.
  3. Remove "expr": true: Not sure why it's in there to begin with.

In jshintignore added

core/misc/jquery.ui.touch-punch.js
core/misc/domready

That would make .jshintrc like this:

{
  "browser"   : true,
  "curly"     : true,
  "eqeqeq"    : true,
  "forin"     : true,
  "latedef"   : true,
  "newcap"    : true,
  "noarg"     : true,
  "strict"    : true,
  "trailing"  : true,
  "undef"     : true,
  "unused"    : "vars",
  "predef"    : [
    "Drupal",
    "drupalSettings",
    "domready",
    "jQuery",
    "_",
    "matchMedia",
    "Backbone",
    "Modernizr",
    "VIE",
    "CKEDITOR"
  ]
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_’s picture

FileSize
3.38 KB

That's make us go from 15 errors currently, to 38 :)

nod_’s picture

Status: Needs review » Reviewed & tested by the community

Nothing much to discuss here. The goal is to make the configuration as strict as possible without getting in the way. Now that most of core is fixed we can be a bit stricter. I think the current patch will be as strict as we can get.

Wim Leers’s picture

Configuration changes: all sensible.

Tested manually: works as expected.

RTBC +1

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed cd59b94 and pushed to 8.x. Thanks!

droplet’s picture

Status: Fixed » Needs review
FileSize
205 bytes

VIE.js is removed ?

Quick patch, if needed i will open a new issue.

nod_’s picture

Status: Needs review » Reviewed & tested by the community

oh that's right! I forgot about it.

I'm fine with keeping this issue for updating the config over time. Unless core committers prefer a new issue every time.

Wim Leers’s picture

Status: Reviewed & tested by the community » Needs work

Likewise, core/misc/create and core/misc/vie should be removed from .jshintignore.

nod_’s picture

Status: Needs work » Needs review
FileSize
645 bytes

good catch

Status: Needs review » Needs work

The last submitted patch, core-jshint-config-update-1995996-8.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
nod_’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed ee5c2c1 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.