1508817 - Enable ESLint for docshell

Status: RESOLVED FIXED

(Core :: DOM: Navigation, enhancement, P3)

Description

[Mark Banner (:standard8)]

As part of rolling out ESLint across the tree, we should enable it for the rest of docshell, i.e. all of:

docshell/test/browser/**
docshell/test/iframesandbox/**
docshell/test/mochitest/**

Comment 1

[sathia]

I'll work on this issue if no one has yet

Comment 2

[sathia]

Attachment: Bug 1508817 - Enable ESLint for {docshell/test/browser/**}/(automatic changes) {docshell/test/iframesandbox/**}/(automatic changes) {docshell/test/mochitest/**}/(automatic changes)

Comment 3

[Mark Banner (:standard8)]

I've not heard from Sathia, so opening this out as a mentored bug.

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: https://developer.mozilla.org/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build
   • If you have any problems, please ask on IRC (https://wiki.mozilla.org/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: https://developer.mozilla.org/docs/Mozilla/Developer_guide/Introduction
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.
   • Here's what to do:
     1. In .eslintignore, remove the three docshell/test/... lines.
     2. Run eslint ./mach eslint --fix docshell/test
        • 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 docshell at the end (this avoids committing .eslintignore at this stage)
        • $ hg commit -m "Bug nnn - Enable ESLint for docshell (automatic changes). r?Standard8" docshell
     5. For the remaining issues, you'll need to fix them by hand. To find them, run ./mach eslint docshell.
        • Most of those should be reasonably easy to understand, but there's more information on some specific bits (especially no-undef) here:
          • https://developer.mozilla.org/docs/Mozilla/Developer_guide/ESLint#Common_issues_and_how_to_solve_them
          • http://eslint.org/docs/rules/
     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 docshell (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 docshell 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 4

[Mellina Y.]

Hi Mark,
I could work on this, if that's ok :)

Thanks!

Comment 5

[Mark Banner (:standard8)]

Hi Mellina, I've assigned it to you, please start working on it. Feel free to ask questions.

Comment 6

[Mellina Y.]

Hi Mark,

Thanks! I will work on it today and will have updates tomorrow.

Comment 7

[Mellina Y.]

Attachment: Bug 1508817 - Enable ESLint for docshell (automatic changes). r?Standard8

Comment 8

[Mellina Y.]

Attachment: Bug 1508817 - Enable ESLint for docshell (manual changes). r?Standard8

Depends on D23037

Comment 9

[Mark Banner (:standard8)]

Hi Mellina, sorry I haven't gotten to look at this yet - I've had lots on the last day or so. I hope to look at it tomorrow if not before.

Comment 10

[Mellina Y.]

Hi Mark, don't worry! There is a lot to check here as well, I understand.

I just gave a heads-up in case you haven't seen it.

Please take your time.

Comment 11

[Mark Banner (:standard8)]

Boris, can you accept the automatic changes as well please?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Sorry, I had missed there were two requests, somehow. Done.

Comment 13

[Mellina Y.]

Attachment: Bug 1508817 - Enable ESLint for docshell (automatic changes). r?Standard8

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b338bce4319e
Enable ESLint for docshell (automatic changes). r=Standard8,bzbarsky
https://hg.mozilla.org/integration/autoland/rev/cb0dd2254bc3
Enable ESLint for docshell (manual changes). r=Standard8,bzbarsky

Comment 15

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b338bce4319e
https://hg.mozilla.org/mozilla-central/rev/cb0dd2254bc3