As part of rolling out ESLint across the tree, we should enable it for all of
To help Mozilla out with this bug, here's the steps:
- Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
- 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.
- 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:
- In .eslintignore, remove the lines that start with
devtools/shared/ in the inclusions section - see this link for the exact lines.
- Run eslint
./mach eslint --fix devtools/shared/
- This should fix some of the issues.
- Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok.
- Create a commit of the work so far. Note the extra
devtools/shared/ at the end (this avoids committing .eslintignore at this stage)
$ hg commit -m "Bug nnn - Enable ESLint for all of devtools/shared/ (automatic changes). r?Standard8" devtools/shared/
- For the remaining issues, you'll need to fix them by hand. To find them, run
./mach eslint devtools/shared/.
- To work around
mozilla/no-aArgs issues, please find the
mozilla/no-aArgs section in devtools/.eslintrc.js and add to the
files list the filenames where the rule is shown as failing.
- These will be fixed in a future bug, and limiting it to just the failing instances prevents more files being added with more failures.
- 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 and here.
- Test your changes, run
$ ./mach mochitest devtools/shared/tests/mochitest/ devtools/shared/webconsole/ devtools/shared/qrcode/tests/mochitest/
- 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/shared/ (manual changes). r?Standard8"
- Post the two commits via phabricator. Please use moz-phab to submit them.
- 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.
- 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!
- 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.