refactor: add @cordova/eslint-config dependency w/ lint fixes #95
Conversation
This is applied first before applying eslint's automated fix. With vars consolidated and split on new lines, eslint automated fix will correct the first line and leave the other lines untouched causing alot of variables not being declared. This will help the linting process.
NiklasMerz
left a comment
NiklasMerz
left a comment
There was a problem hiding this comment.
I did not read every line but changes look good. Tests pass locally and in CI 👍
raphinesse
left a comment
raphinesse
left a comment
There was a problem hiding this comment.
I just peeked in and left a few comments. Definitely too many changes for me to review properly right now 😅
Would definitely be good to have this repo linted too.
| return extension; | ||
| for (var extension in FILETYPE_BY_EXTENSION) { | ||
| if (FILETYPE_BY_EXTENSION.hasOwnProperty(unquoted(extension))) { | ||
| if (FILETYPE_BY_EXTENSION[unquoted(extension)] === unquoted(filetype)) { return extension; } |
There was a problem hiding this comment.
Ugh, why does the auto-fix format that onto one line 😒 There are a few instances of changes like this. No deal breakers but not pretty either.
|
|
||
| for (var index = 0; index < dependencyTargets.length; index++) { | ||
| var dependencyTargetUuid = dependencyTargets[index]; | ||
| for (var index2 = 0; index2 < dependencyTargets.length; index2++) { |
There was a problem hiding this comment.
Violations like these would disappear later when transforming var to let. Having index2 as a variable name could be irritating.
brody4hire
left a comment
brody4hire
left a comment
There was a problem hiding this comment.
My apologies for the late review. I have a feeling that changing one thing at a time in separate PRs would introduce extra code churn not needed in the master branch. I would personally favor making one PR per source (or test) module.
I am also not so thrilled about combining package-lock.json with this PR.
Thanks @erisu for all of your hard work on this, apologies for the difficulty.
|
Closing PR.
Also making a note that this PR never commited package-lock.json. |
4 participants
@cordova/eslint-configdependency