Removes jsHint in favour of ESLint. Adds Babel-Runtime polyfill and t… #293
Conversation
|
The syntax standard in the Origami spec will also need to be updated. |
|
Pinging @triblondon @matthew-andrews to check new linting rules |
| return function(file) { | ||
| if (!(/\.js$/).test(file)) return through(); | ||
|
|
||
| var rootDirectory = path.resolve(path.normalize(config.rootDirectory || './node_modules')); |
There was a problem hiding this comment.
What is ./node_modules relative to here?
Might be better to throw if rootDirectory isn't defined.
There was a problem hiding this comment.
It's relative to that file. I agree throwing would be better
|
See comment, otherwise 👍 |
|
Please note, eslint 1.0.0 and gulp-eslint 1.0.0 have been released in the last few days. There are some breaking changes, but also many bug fixes. Let me know if you have any questions about ESLint and I'll do my best to answer. |
let npm publish obt with `.eslintrc`
don't allow overwriting of globals [consistent with obt]
| "no-multi-str": 0, | ||
| "dot-notation": 0, | ||
| "strict": [2, "function"], | ||
| "quotes": [1, "single"], |
There was a problem hiding this comment.
Very much in favour of removing the warning, it's quite noisy. I wanted to test it out, it's quite nice to have some kind of convention on modules, but after running verify on a few modules, it spilled out dozens and dozens of warnings. I'll probably keep it in obt though
There was a problem hiding this comment.
Not sure if it's helpful, but if you want to keep the warnings but not see them all the time, you can run ESLint with --quiet which will prevent display of warnings.
Also, if you're getting a lot of errors/warnings, I made eslint-nibble to help sort through them and make it less overwhelming.
There was a problem hiding this comment.
@IanVS regarding --quiet, what's the difference between that and just removing the rule? And also, does that option exist in gulp-eslint, doesn't seem to
There was a problem hiding this comment.
Ah that's right, forgot you are using gulp-eslint. I haven't used that, I typically just create an npm script and run eslint before my tests that way.
The difference with quiet would be that you could still run without it and see the output without having to change the config, which is useful if it's your goal to clean it up eventually.
There was a problem hiding this comment.
Right, thanks, I think I'm going to get in less fights if I just remove the rule altogether.
|
@matthew-andrews I agree with leaving things to lintspaces, I just tried to copy rules over and then open up the discussion. Also, you should be checking out the file in the config directory, you're looking at the one used in the tests |
|
Oops, thanks :) |
| configFile: configPath | ||
| }), | ||
| eslint.format('compact', process.stdout), | ||
| eslint.failAfterError() |
There was a problem hiding this comment.
hmm we sure we want this?
// To have the process exit with an error code (1) on // lint error, return the stream and pipe to failOnError last.
if you're pulling this in as a dependency and running it from inside node this behaves very oddly… is this how it worked in jshitn?
There was a problem hiding this comment.
What do you mean it behaves oddly? What's the behaviour you would expect? JSHint had a failOnError or similar function, and so does ScssLint
|
I've released a new beta, rules should be final, only concern is this |
…ng to compile non .js files
… latter still does not work due to webpack issue
…o work with textrequireify and allowing to pass options to it
…ot found errors and syntax errors
| "exports": false, | ||
| "requireText": false | ||
| } | ||
| } |
There was a problem hiding this comment.
Is this the same as config/.eslintrc? If so do we have to duplicate?
There was a problem hiding this comment.
No, it has a difference, that way we can test that it's using the custom config instead of the default one in its corresponding test
Removes jsHint in favour of ESLint. Adds Babel-Runtime polyfill and t…
…ransformer to fix paths
DO NOT MERGE
So this would be a new major release. We would get Babel polyfills, and change to ESLint, which changes linting rules which will affect, probably, all projects. Rules should be discussed before merging. Tests are passing, but not verify as I would like to finalise the linting rules for both obt, and modules.
@triblondon @matthew-andrews