Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(533)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years ago by hub
Modified:
3 years ago
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
3 years ago (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 ...
3 years ago (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, ...
3 years ago (2017-05-19 16:47:31 UTC) #3
Wladimir Palant
3 years ago (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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5