Skip to content

Conversation

@impaler
Copy link
Member

@impaler impaler commented Mar 19, 2015

@bholloway @bguiz the remaining jshint errors or any changes I made may need discussion around the existing rules.

You can run the jshint from cli with npm run lint or they will also run with the specs in npm test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'join_vars': false

possible?

tasks/watch.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the rest of this file, if there are no references to config, we can drop this statement. It was probably made redundant as part of the refactor making things more DRY.

@bguiz
Copy link
Contributor

bguiz commented Mar 19, 2015

Looks like the freshly merged in integrations tests, almost all the tests are failing due to this error:

TypeError: Object #<Object> has no method 'addMatchers'

- also added a new npm run task for `testonly` that doesn't run lint, and isn't verbose
@bguiz
Copy link
Contributor

bguiz commented Mar 19, 2015

I reproduced the above error on Travis locally with jasmine-node@1.x.xand upgrade to jasmine-node@2.x.x did the trick. Let's figure out why devDependecnies aren't updating on Travis

@bguiz
Copy link
Contributor

bguiz commented Mar 19, 2015

Tailing the Travis logs now, looks like the tests are finally running properly (after some trial and error getting it to work)

@bguiz
Copy link
Contributor

bguiz commented Mar 19, 2015

Great success! ... and relatively fast too, at 6 minutes:
https://travis-ci.org/angularity/node-angularity/builds/54983907

W: http://bguiz.com

On 19 March 2015 at 15:53, Chris notifications@github.com wrote:

@bholloway https://github.com/bholloway @bguiz
https://github.com/bguiz the remaining jshint errors or any changes I
made may need discussion around the existing rules.

You can run the jshint from cli with npm run lint or they will also run

with the specs in npm test.

You can view, comment on, or merge this pull request online at:

#31
Commit Summary

  • Add linting to the npm test script.
  • jshint fixes cli.js.
  • jshint fixes build.js.
  • jshint fixes css.js.
  • jshint fixes html.js.
  • jshint fixes init.js.
  • jshint fixes javascript.js.
  • jshint fixes release.js.
  • jshint fixes server.js.
  • jshint fixes test.js.
  • jshint fixes watch.js.
  • jshint fixe webstorm.js.
  • Adjust jshint configuration to allow javascript url syntax that
    conflicts with gulp task definitions.
  • jshint fixes for specs.
  • jshint fixes for browserify.js.
  • jshint fixes for node-sass.js.
  • jshint fixes for defaults.js.
  • jshint fixes for platform.js.
  • jshint fixes for streams.js.
  • jshint fixes for adjacent-files.js.
  • jshint fixes for brower-files.js.
  • jshint fixes for karma.js.
  • jshint fixes for hr.js.
  • jshint fixes for jshint-reporter.js.
  • jshint fixes for task-yargs-run.js.
  • jshint fixes for task-yargs.js.
  • jshint fixes for yargs.js.
  • Remove interactive menus, they maybe reimplemented in a future
    version.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#31.

@impaler
Copy link
Member Author

impaler commented Mar 19, 2015

@bguiz cheers for the fixes, its crappy how much slower appveyor is with windoze!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not as intended. We want the final method above to be the fall-through. As it stands, it will be added always.

bholloway added a commit that referenced this pull request Mar 20, 2015
Jshint fixes + Fixed CI problems
@bholloway bholloway merged commit a69f0a0 into master Mar 20, 2015
@bholloway bholloway deleted the jshint branch March 24, 2015 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants