jscs check should fail the tests

adborden commented 9 years ago

jscs check should fail the tests:

$ npm test

> AngularJS-Gulp-Browserify-Starter@1.2.0 test /home/adborden/projects/brigade/disclosure-frontend-alpha
> gulp test

[19:57:29] Using gulpfile ~/projects/brigade/disclosure-frontend-alpha/gulpfile.js
[19:57:29] Starting 'build-test'...
[19:57:29] Starting 'clean-full'...
[19:57:29] Finished 'clean-full' after 2.62 ms
[19:57:29] Starting 'lint'...
[19:57:29] Starting 'checkstyle'...
Error in plugin 'gulp-jscs'
Message:
    Illegal trailing whitespace at components/common/appMainNav/appMainNav.js :
     5 |  var appMainNavDirective = require('./appMainNavDirective');
     6 |  var AppMainNavController = require('./AppMainNavController');
     7 |
----------^
     8 |  module.exports = angular.module('appMainNavModule', ['ui.bootstrap.collapse'])
     9 |    .directive('appMainNav', appMainNavDirective)
[19:57:30] Finished 'checkstyle' after 443 ms
[19:57:30] Finished 'lint' after 471 ms
[19:57:30] Starting 'karma'...
INFO [framework.browserify]: 6028374 bytes written (8.93 seconds)
INFO [framework.browserify]: bundle built
INFO [karma]: Karma v0.12.37 server started at http://localhost:9876/
INFO [launcher]: Starting browser PhantomJS
INFO [PhantomJS 1.9.8 (Linux 0.0.0)]: Connected on socket IaNj8P8BJvLO8B18VVLD with id 41275141
PhantomJS 1.9.8 (Linux 0.0.0) LOG LOG: 'WARNING: Tried to load angular more than once.'

  MainController
    - should exist

  HomeController
    ✓ should exist

PhantomJS 1.9.8 (Linux 0.0.0): Executed 1 of 2 (skipped 1) SUCCESS (0.011 secs / 0 secs)
TOTAL: 1 SUCCESS

[19:57:43] Finished 'karma' after 13 s
[19:57:43] Finished 'build-test' after 13 s
[19:57:43] Starting 'test'...
[19:57:43] Finished 'test' after 7.06 μs

tdooner commented 9 years ago

@adborden We just discussed this a bit. @jnmarcus has some concerns that we will not be able to tune the checks appropriately, however there is general consensus here that we should turn it on and address developer annoyances on a case-by-case basis.

Also, we should make sure that gulp doesn't crash or stop watching in development mode if it hits a style problem.

adborden commented 9 years ago

Yep, I think we should split the gulp task so the linting is part of the tests or something. Then linting can fail the tests, but on development, it just spits out warnings (like it does now).

bcipolli commented 8 years ago

@adborden was this fixed by any of the changes you made last night? if not, is this something you could address? Seems important to me, especially if we're not doing code reviews before merging.

bcipolli commented 8 years ago

@adborden Can you make this happen? Or did this get in, in one of your recent PRs?

KyleW commented 8 years ago

Since I'm messing with tests and infrastructure, I can grab this. Is there any CI system in place? Or is the goal just to exit the gulp build-prod task with a non-zero exit code?

Edit: oh Travis. I see . . .read the docs and all.

KyleW commented 8 years ago

Putting this off to focus on features for the moment. Will reconsider once the MVP is out.

adborden commented 8 years ago

:thumbsup: I'm going to un-assign you to help communicate who is working on what.

caciviclab / disclosure-frontend

jscs check should fail the tests #13