fix(findBin): Add separator before npm args #297
Conversation
Codecov Report
@@ Coverage Diff @@
## master #297 +/- ##
==========================================
- Coverage 91.9% 91.86% -0.05%
==========================================
Files 10 10
Lines 173 172 -1
Branches 27 26 -1
==========================================
- Hits 159 158 -1
Misses 13 13
Partials 1 1
Continue to review full report at Codecov.
|
| @@ -8,25 +8,25 @@ describe('findBin', () => { | |||
| it('should favor `npm run` command if exists in both package.json and .bin/', () => { | |||
| const { bin, args } = findBin('my-linter', { 'my-linter': 'my-linter' }) | |||
| expect(bin).toEqual('npm') | |||
| expect(args).toEqual(['run', '--silent', 'my-linter']) | |||
| expect(args).toEqual(['run', '--silent', 'my-linter', '--']) | |||
There was a problem hiding this comment.
Can we only add this if there are arguments?
There was a problem hiding this comment.
I thought of that.. we need it cause we pass file names later on.
There was a problem hiding this comment.
Oh gotcha. We should a comment then for future us :)
| @@ -24,16 +24,14 @@ module.exports = function runScript(commands, pathsToLint, scripts, config) { | |||
| try { | |||
| const res = findBin(linter, scripts, config) | |||
|
|
|||
| const separatorArgs = /npm(\.exe)?$/i.test(res.bin) ? ['--'] : [] | |||
There was a problem hiding this comment.
It’s good this is gone. I hated it here
|
LGTM but see my comments |
|
@okonet Shall we merge this in? |
|
Merging this in as @okonet is probably busy at the conference. |
|
Thanks! |
|
Ok. I'll add a comment. |
Closes #296.