Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use shorthand lint target from test #6406

Merged
merged 1 commit into from
Apr 30, 2016

Conversation

jbergstroem
Copy link
Member

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

build

Description of change

Instead of invoking jslint/cpplint from the test target, call on the generic lint instead, since it checks if eslint exists. Since our tarballs lacks eslint we get a more graceful exit from make test instead of a traceback from jslint.

/cc: @nodejs/build

@jbergstroem jbergstroem added the build Issues and PRs related to build files or the CI. label Apr 27, 2016
@jbergstroem
Copy link
Member Author

jbergstroem commented Apr 27, 2016

CI: https://ci.nodejs.org/job/node-test-commit/3067/

Edit:

  • osx fail in tests (test-tick-processor.js): unrelated

@rmg
Copy link
Contributor

rmg commented Apr 27, 2016

LGTM

@cjihrig
Copy link
Contributor

cjihrig commented Apr 27, 2016

LGTM

2 similar comments
@addaleax
Copy link
Member

LGTM

@jasnell
Copy link
Member

jasnell commented Apr 27, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Apr 29, 2016

@jbergstroem ... this needs to be rebased to land.

Instead of invoking jslint/cpplint from the test target,
call on the generic lint instead since it checks if eslint
exists. Since our tarballs lacks eslint we now get a more graceful
exit from `make test` instead of a traceback from jslint.

PR-URL: nodejs#6406
Fixes: nodejs#6408
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jbergstroem jbergstroem merged commit 62c31fe into nodejs:master Apr 30, 2016
Fishrock123 pushed a commit that referenced this pull request May 4, 2016
Instead of invoking jslint/cpplint from the test target,
call on the generic lint instead since it checks if eslint
exists. Since our tarballs lacks eslint we now get a more graceful
exit from `make test` instead of a traceback from jslint.

PR-URL: #6406
Fixes: #6408
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request May 4, 2016
Instead of invoking jslint/cpplint from the test target,
call on the generic lint instead since it checks if eslint
exists. Since our tarballs lacks eslint we now get a more graceful
exit from `make test` instead of a traceback from jslint.

PR-URL: nodejs#6406
Fixes: nodejs#6408
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

@jbergstroem lts?

@MylesBorins
Copy link
Contributor

ping @jbergstroem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants