Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29441570: Issue 5257 - Add an eslint target to npm run and run it posttest. (Closed)

Created:
May 18, 2017, 3:53 p.m. by hub
Modified:
May 19, 2017, 6:25 p.m.
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Issue 5257 - Add an eslint target to npm run and run it posttest.

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -1 line) Patch
M package.json View 1 chunk +5 lines, -1 line 3 comments Download

Messages

Total messages: 4
hub
May 18, 2017, 3:53 p.m. (2017-05-18 15:53:23 UTC) #1
Wladimir Palant
https://codereview.adblockplus.org/29441570/diff/29441571/package.json File package.json (right): https://codereview.adblockplus.org/29441570/diff/29441571/package.json#newcode15 package.json:15: "eslint": "eslint *.js lib test chrome", IMHO, this will ...
May 19, 2017, 3:52 p.m. (2017-05-19 15:52:31 UTC) #2
hub
https://codereview.adblockplus.org/29441570/diff/29441571/package.json File package.json (right): https://codereview.adblockplus.org/29441570/diff/29441571/package.json#newcode15 package.json:15: "eslint": "eslint *.js lib test chrome", On 2017/05/19 15:52:31, ...
May 19, 2017, 4:47 p.m. (2017-05-19 16:47:31 UTC) #3
Wladimir Palant
May 19, 2017, 6:12 p.m. (2017-05-19 18:12:31 UTC) #4
LGTM

This needs to land on master and emscripten branches.

https://codereview.adblockplus.org/29441570/diff/29441571/package.json
File package.json (right):

https://codereview.adblockplus.org/29441570/diff/29441571/package.json#newcode15
package.json:15: "eslint": "eslint *.js lib test chrome",
On 2017/05/19 16:47:31, hub wrote:
> On 2017/05/19 15:52:31, Wladimir Palant wrote:
> > IMHO, this will run the global ESLint and not the one you added as
dependency
> > here.
> 
> $ which eslint
> eslint not found
> 
> If I echo $PATH from the npm test command, I get the output of `npm bin` ahead
> of the system directories, like /usr/local/bin.
> 
> So it will definitely use the one npm just installed locally.

Good to know, definitely saves some complexity here.

Powered by Google App Engine
This is Rietveld