Bug 1589333 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

As part of rolling out ESLint across the tree, we should enable it for all of `devtools/client/shared/`.

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](https://developer.mozilla.org/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build). An artifact build is all you need.
    * 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](https://developer.mozilla.org/docs/Mozilla/Developer_guide/Introduction), 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](https://searchfox.org/mozilla-central/rev/97976753a21c1731e18177de9e5ce78ea3b3da2d/.eslintignore#63-65), remove the lines that start with `devtools/client/shared/` and the lines below it that start with `!devtools/client/shared/`.
      2. Run eslint `./mach eslint --fix devtools/client/shared/`
          * 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 `devtools/client/shared/` at the end (this avoids committing .eslintignore at this stage)
          * `$ hg commit -m "Bug nnn - Enable ESLint for all of devtools/client/shared/ (automatic changes). r?Standard8" devtools/client/shared/`
      5. For the remaining issues, you'll need to fix them by hand. To find them, run `./mach eslint devtools/client/shared/`.
          * no-unused-vars: These generally fall into one of two cases:
            * `const foo = bar()` - if it is like this, it is generally alright to change to `bar()`. Although the `foo` is not use, the assignment side is sometimes still required for side-effects.
            * Sometimes a function is shown as unused, but is referenced in onload functions, in this case you can add a line saying `/* exported bar */` which will satisfy ESLint.
          * Most of the rules 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) and [here](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 Enable ESLint for all of devtools/client/shared/ (manual changes). r?Standard8"`
      7. Post the two commits via [phabricator](https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html). **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 devtools peer, so there may still be more to go.
  * If you do need to update the patches, please amend the existing commits, rather than creating new ones. This helps with tracking of review comments.
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.
As part of rolling out ESLint across the tree, we should enable it for all of `devtools/client/shared/`.

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](https://developer.mozilla.org/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build). An artifact build is all you need.
    * 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](https://developer.mozilla.org/docs/Mozilla/Developer_guide/Introduction), 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](https://searchfox.org/mozilla-central/rev/97976753a21c1731e18177de9e5ce78ea3b3da2d/.eslintignore#63-65), remove the lines that start with `devtools/client/shared/` and the lines below it that start with `!devtools/client/shared/`.
      2. Run eslint `./mach eslint --fix devtools/client/shared/`
          * 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 `devtools/client/shared/` at the end (this avoids committing .eslintignore at this stage)
          * `$ hg commit -m "Bug nnn - Enable ESLint for all of devtools/client/shared/ (automatic changes). r?Standard8" devtools/client/shared/`
      5. For the remaining issues, you'll need to fix them by hand. To find them, run `./mach eslint devtools/client/shared/`.
          * no-unused-vars: These generally fall into one of two cases:
            * `const foo = bar()` - if it is like this, it is generally alright to change to `bar()`. Although the `foo` is not use, the assignment side is sometimes still required for side-effects.
            * Sometimes a function is shown as unused, but is referenced in onload functions, in this case you can add a line saying `/* exported bar */` which will satisfy ESLint.
          * Most of the rules 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) and [here](http://eslint.org/docs/rules/).
      6. Test your changes, run `$ ./mach mochitest devtools/client/shared`
      7. 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 Enable ESLint for all of devtools/client/shared/ (manual changes). r?Standard8"`
      8. Post the two commits via [phabricator](https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html). **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 devtools peer, so there may still be more to go.
  * If you do need to update the patches, please amend the existing commits, rather than creating new ones. This helps with tracking of review comments.
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.

Back to Bug 1589333 Comment 0