[wp-trac] [WordPress Trac] #26446: Travis CI should run jshint

[WordPress Trac]

#26446: Travis CI should run jshint
-------------------------+------------------------------
 Reporter:  jorbin       |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Awaiting Review
Component:  Build Tools  |     Version:
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |
-------------------------+------------------------------

Comment (by jorbin):

 Replying to [comment:1 bpetty]:
 > Are you sure you want Travis to send notifications every time a commit
 is made that violates jshint rules rather than just notifications when the
 code actually fails to function properly?

 One of the things jshint gives us is an indication that our code might not
 function properly.  All The ES3 warnings tell us of potential problems in
 some of the browsers we support.

 >
 > Not everyone realizes this at first, but there is a line you want to
 draw here. Add too much into Travis, and everyone gets used to (and starts
 to ignore) build failure notifications. I'd also hate to see committers
 getting into the habit of committing first, and relying on Travis to run
 the tests for them after the fact rather than running the checks before
 they commit.
 >

 I agree there is a line to draw, but I think that drawing it at only unit
 tests is drawing too short. Unit testing failures should be a show
 stopper, that causes the developers that worked on the breaking change to
 go back and fix it. JSHint is the same way. When it doesn't pass, we
 should figure out why and fix it.


 > Travis mostly helps ease the pain of testing across multiple
 environments while pin-pointing exactly which commit broke it at the same
 time. I don't think we really need that to check for jshint violations
 though. You already know the location, and the test doesn't need to be run
 across multiple build environments. It's simple enough to run on your
 local dev machine because you only need to check in one environment, and
 it currently only takes 16 seconds to run.

 I would love to figure out a way to not have to run jshint for each of the
 configurations since you are correct that it doesn't need to be run across
 multiple build environments.  I'm not sure if Travis supports that, but I
 imagine we can code around that if you really think it is a show stopper.
 Of course, as you point out we are only adding ~16 seconds to the travis
 build.

--
Ticket URL: <http://core.trac.wordpress.org/ticket/26446#comment:2>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software