1344286 - Fix ESLint issues in devtools/client/memory/test/chrome/*.html

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Comment 1

[Tim Nguyen :ntim]

Attachment: WIP

Comment 2

[Hemanth Kumar Veeranki [:harry7]]

Hi,
I would like to work on this bug. Can you assign this to me. Also can someone guide me on what is to be done and where I should start looking at?
Thanks,

Comment 3

[Tim Nguyen :ntim]

(In reply to Hemanth Kumar Veeranki [:harry7] from comment #2)
> Hi,
> I would like to work on this bug. Can you assign this to me. Also can
> someone guide me on what is to be done and where I should start looking at?
> Thanks,


[0] provides a list of ignored files by ESLint in the devtools/ folder. 

Steps:
- Run `./mach eslint --no-ignore ` followed by one of the lines in [0]
- Once you do this, you will get a list of JS errors/warnings, you will need to fix them one by one.
- Once you're done fixing the errors, you can remove the relevant line in [0].

Note that you don't need to do all the lines from [0] in the same bug. Feel free to only fix a couple of lines.

[0]: https://dxr.mozilla.org/mozilla-central/source/.eslintignore#104-125

Comment 4

[Hemanth Kumar Veeranki [:harry7]]

(In reply to Tim Nguyen :ntim from comment #3)

> Steps:
> - Run `./mach eslint --no-ignore ` followed by one of the lines in [0]
When I ran the following command with one of the files, I am getting an error like this
> mach: error: unrecognized arguments for lint: '--no-ignore'

I am not able to proceed further. What should I do next?

Comment 5

[Hemanth Kumar Veeranki [:harry7]]

I ran eslint like `./mach lint -l eslint` instead of `./mach eslint` and it worked.
Thanks

Comment 6

[Hemanth Kumar Veeranki [:harry7]]

Attachment: Bug 1344286 - Fix eslint errors from devtools files ignored previously;

Review commit: https://reviewboard.mozilla.org/r/251310/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/251310/

Comment 7

[Greg Tatum [:gregtatum]]

Comment on attachment 8985769 [details]
Bug 1344286 - Fix eslint errors from devtools files ignored previously;

https://reviewboard.mozilla.org/r/251310/#review262504

All the changes look correct to me, I'm running a try run now to verify. Thanks for the patch!

Comment 8

[Tim Nguyen :ntim]

You might want to remove https://searchfox.org/mozilla-central/source/.eslintignore#104 before landing :)

Comment 9

[Greg Tatum [:gregtatum]]

Woops, can you do that :harry7? And then needinfo me and I'll re-run the the try server and try to land this.

Comment 10

[Hemanth Kumar Veeranki [:harry7]]

Comment on attachment 8985769 [details]
Bug 1344286 - Fix eslint errors from devtools files ignored previously;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/251310/diff/1-2/

Comment 11

[Hemanth Kumar Veeranki [:harry7]]

(In reply to Greg Tatum [:gregtatum] [@gregtatum] from comment #9)
> Woops, can you do that :harry7? And then needinfo me and I'll re-run the the
> try server and try to land this.

Thanks :gregtatum I added that change to the commit and pushed it for review. Should I continue doing these changes in different commits? so that the commit won't become too large?

Comment 12

[Tim Nguyen :ntim]

Comment on attachment 8985769 [details]
Bug 1344286 - Fix eslint errors from devtools files ignored previously;

https://reviewboard.mozilla.org/r/251310/#review262556

The new ESLint push looks green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=320db4ee1c3181579e84be23b2067fc758308545

along with Greg's earlier push (https://treeherder.mozilla.org/#/jobs?repo=try&revision=d716c042b99de2b566df0ebf11695f36298f2396) where devtools tests were green.

Thanks for working on this!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34ff21c524d4
Fix eslint errors from devtools files ignored previously; r=gregtatum,ntim

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/34ff21c524d4

Comment 15

[Greg Tatum [:gregtatum]]

Thanks Hemanth!