respect userOptions.eslintPath #195
Conversation
|
Can you fix failing test please? |
|
@MoOx I edited the test/eslint-path to fix some test check。but still has one test faild. |
|
It's related to a specific version of node/npm with webpack one. You need to check here https://travis-ci.org/MoOx/eslint-loader/builds/289800514 |
|
@lmnsg I think I fixed CI on master branch. Can you rebase your branch on master? Should help :) |
|
@MoOx It's ok. 😆 |
test/eslint-path.js
Outdated
| t.true( | ||
| stats.hasErrors(), | ||
| !stats.hasErrors(), | ||
| "a file that does not contains error but mock eslint instance " + | ||
| "returned error" |
There was a problem hiding this comment.
We should probably update this comment then?
test/eslint-path.js
Outdated
| "a file that does not contains error but mock eslint instance " + | ||
| "returned error" | ||
| ) | ||
| t.true( | ||
| (stats.compilation.errors[0].message + "").indexOf("Fake error") > -1 | ||
| stats.compilation.errors.length === 0 || (stats.compilation.errors[0].message + "").indexOf("Fake error") > -1 |
There was a problem hiding this comment.
Not sure about this test now :)
There was a problem hiding this comment.
In fact, I do not understand this two test too much..😂
especially the last one.
There was a problem hiding this comment.
Actually, test/mock/eslint-mock.js is returning some fake errors. We first check that we have errors via webpack stats api, then we check that the error is indeed our "Fake error" defined in the eslint-mock.
So we should not have to change this tests, otherwise, that mean the mock is unused.
There was a problem hiding this comment.
well, I see.
I will check my code again.
if them specify an eslintPath option, and it like a officer eslint folder. he dont have to install the eslint.
|
@MoOx I re-pushed my code, please review it again... |
| @@ -159,7 +159,8 @@ module.exports = { | |||
|
|
|||
| #### `eslintPath` (default: "eslint") | |||
|
|
|||
| Path to `eslint` instance that will be used for linting. | |||
| Path to `eslint` instance that will be used for linting. | |||
| If the `eslintPath` is a folder like a official eslint, or specify a `formatter` option. now you dont have to install `eslint` . | |||
There was a problem hiding this comment.
Not sure what you are trying to explain here. Can you help me to understand so we can put a better explanation?
There was a problem hiding this comment.
Sure, when someone specify the eslintPath，often means he want use another version of eslint or the eslint under other node_modules.
so he doesn't want to install eslint again in current project/node_modules
|
Released in 2.0.0 |
| @@ -150,13 +150,25 @@ module.exports = function(input, map) { | |||
| loaderUtils.getOptions(this) | |||
| ) | |||
|
|
|||
| var userEslintPath = userOptions.eslintPath | |||
| var formatter = require("eslint/lib/formatters/stylish") | |||
There was a problem hiding this comment.
Is this require("eslint/lib/formatters/stylish") redundant given the one in the try-catch below?
#user should not depend eslint/lib/formatters/stylish if he appoint an eslintPath.