1532933 - Enable ESLint for dom/xhr

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Mark Banner (:standard8)]

As part of rolling out ESLint across the tree, we should enable it for dom/xhr.

To help Mozilla out with this bug, here's the steps:

1. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
2. Download and build the Firefox source code, see the docs for details. An artifact build is all you need.
   • If you have any problems, please ask on IRC in the #introduction channel. They're there to help you get started.
   • You can also read the Developer Guide, which has answers to most development questions.
3. Start working on this bug
   • Please note:
     • We want to end up with two separate commits. One with the automatic changes, the second with the manual changes.
     • Although you'll change .eslintignore at the start, only the second commit should have the .eslintignore changes. Please follow the suggested commands closely.
   • Here's what to do:
     1. In .eslintignore, remove the dom/xhr/** line.
     2. Run eslint ./mach eslint --fix dom/xhr
        • This should fix some of the issues.
     3. Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok.
     4. Create a commit of the work so far. Note the extra dom/xhr at the end (this avoids committing .eslintignore at this stage)
        • $ hg commit -m "Bug nnn - Enable ESLint for dom/xhr (automatic changes). r?Standard8" dom/xhr
     5. For the remaining issues, you'll need to fix them by hand. To find them, run ./mach eslint dom/xhr.
        • Most of those should be reasonably easy to understand, but there's more information on some specific bits (especially no-undef) here and here.
     6. Create a second commit of the manual changes, note, there's no directory specifier this time, so .eslintignore will be included.
        • $ hg commit -m "Bug nnn - Enable ESLint for dom/xhr (manual changes). r?Standard8
     7. Post the two commits via phabricator. Please use moz-phab to submit them.
4. Once the patches are submitted, I'll take a look. If there's any changes necessary I'll comment in Phabricator, so be prepared to update the patches. If there's no changes, I'll request review from a dom peer, so there may still be more to go.
5. Once we're happy with the changes, I'll push it to autoland - our integration branch. Your code will then soon be shipping to Firefox users worldwide!
6. Now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.

Comment 1

[maximillian.widjaja]

Hi Mark Banner,

I would like to help to fix this bug.

Best,

Max

Comment 2

[Mark Banner (:standard8)]

Hi Max, please do start working on it. Feel free to ask if you have questions.

Comment 3

[maximillian.widjaja]

Hi Mark,

I removed dom/xhr/** line in step a.
In step b, I ran ./mach eslint --fix dom/xhr. I got a lot of errors.
What do you mean by 'This should fix some of the issues'?
From here, do I continue to step c and d, or fix the errors and then move on?

Thanks,

Max

Comment 4

[Shivam Singhal [ :championshuttler ]]

Hi Max,

I removed dom/xhr/** line in step a.
In step b, I ran ./mach eslint --fix dom/xhr. I got a lot of errors.
What do you mean by 'This should fix some of the issues'?
From here, do I continue to step c and d, or fix the errors and then move on?

Yes you need to continue with Steps C and D.

After ./mach eslint --fix dom/xhr you can check some changes happened with hg diff and you need to commit them with the commit message mentioned in Step D and after that rest of the errors you are getting you need to solve manually one by one and then commit them with the commit message mentioned in Step F. You can check the errors by running ./mach eslint dom/xhr at root of the cloned source code. Let us know if you need more help.

Comment 5

[Shashank Kaushik]

Hi Mark,

I would like to take up this enhancement.

Thanks,
Shashank Kaushik

Comment 6

[Mark Banner (:standard8)]

Max isn't working on this any more, so unassigning. Shanshank - if you want to take this on after your current bug, let me know.

Comment 7

[Shashank Kaushik]

(In reply to Mark Banner (:standard8) from comment #6)

Max isn't working on this any more, so unassigning. Shanshank - if you want to take this on after your current bug, let me know.

Sure Mark. I will take this up after the current one.
Thanks

Comment 8

[Shashank Kaushik]

Attachment: Bug 1532933 - Enable ESLint for dom/xhr (automatic changes). r?Standard8

Comment 9

[Shashank Kaushik]

Attachment: Bug 1532933 - Enable ESLint for dom/xhr (manual changes). r?Standard8

Depends on D33759

Comment 10

[Shashank Kaushik]

Attachment: Bug 1532933 - Review comments addressed. r?Standard8

Depends on D36437

Comment 11

[Shashank Kaushik]

Attachment: Bug 1532933 - Enable ESLint for dom/xhr (automatic changes). r?Standard8

Comment 12

[Shashank Kaushik]

Attachment: Bug 1532933 - Enable ESLint for dom/xhr (manual changes). r?Standard8

Depends on D45680

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2422a9d771bd
Enable ESLint for dom/xhr (automatic changes). r=Standard8,baku
https://hg.mozilla.org/integration/autoland/rev/a4a76bf71ccc
Enable ESLint for dom/xhr (manual changes). r=Standard8,baku

Comment 14

[Mark Banner (:standard8)]

Thank you for completing this Shashank, it is now landed on our integration branch, and if no issues are raised, it should be merged to mozilla-central within about 24 hours.

Comment 15

[Shashank Kaushik]

Thanks @mark banner . Please feel free assign me the next ticket.

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2422a9d771bd
https://hg.mozilla.org/mozilla-central/rev/a4a76bf71ccc