Extend eslint to test files #3473
Conversation
| @@ -141,7 +143,21 @@ function lintTask() { | |||
| rules: { | |||
| 'complexity': [1, 6], | |||
| 'max-statements': [1, 30] | |||
| } | |||
| }, | |||
| globals: [ | |||
There was a problem hiding this comment.
do you think any problems will occur from having the globals defined for the src as well?
There was a problem hiding this comment.
I doubt it, there were no previous issues with those globals so it would only affect future changes. An alternative would be to run a lint on the source and tests separately.
There was a problem hiding this comment.
yeah. running separately is an option but I've found that it can slow down gulp if the tasks aren't run in parallel
There was a problem hiding this comment.
I would definitively keep only one lint task, including all files.
| { b: 290, w: 91, x: 209, y: 419 }, | ||
| { b: 290, w: 91, x: 322, y: 161 }, | ||
| { b: 290, w: 91, x: 436, y: 419 } | ||
| [{b: 290, w: 91, x: 95, y: 161}, |
There was a problem hiding this comment.
That's the only formatting change I don't really like, I prefer the old way because all entries were aligned with a tab. Maybe we should consider to move the first value on a new line:
[
{b: 290, w: 91, x: 95, y: 161},
{b: 290, w: 91, x: 209, y: 419},
// ...
].forEach(...There was a problem hiding this comment.
I think that would be the best way to do it, I will update the PR with that change soon.
| @@ -141,7 +143,21 @@ function lintTask() { | |||
| rules: { | |||
| 'complexity': [1, 6], | |||
| 'max-statements': [1, 30] | |||
| } | |||
| }, | |||
| globals: [ | |||
There was a problem hiding this comment.
I would definitively keep only one lint task, including all files.
| @@ -1,3 +1,6 @@ | |||
| /* eslint no-unused-vars: 1 */ | |||
There was a problem hiding this comment.
With the globals in gulp lint options, do we still need this no-unused-vars: 1?
There was a problem hiding this comment.
For the functions that are defined in lines 80-102, it says that the last parameter is an unused variable. For example, y is unused in lineTo: function(x, y) and h is unused in strokeRect: function(x, y, w, h) {},, etc.
There was a problem hiding this comment.
we could also remove those parameters since they do nothing
|
I will merge by command line to fix the merge conflict when we agree to merge the PR. |
|
Sure, if you do it by command line, can you please rebase and squash your commits before merging fast-forward. Though, would also be handy to rebase and force push your branch to update that PR (keep iteration history), and finally "Squash and merge". |
|
Nice work @zachpanz88! |
* Add eslint to test files * Fix mockContext for tests * Make formatting look better for nested objects
Adds eslint rules for test files so that they follow the same standards as source files which helps to increase overall code style