Closed
Bug 1582658
Opened 5 years ago
Closed 5 years ago
Enable ESLint for all of devtools/client/storage/test
Categories
(DevTools :: Storage Inspector, task)
DevTools
Storage Inspector
Tracking
(firefox71 fixed)
RESOLVED
FIXED
Firefox 71
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: standard8, Assigned: yonashiro.mellina, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [fxsearch][lang=js])
Attachments
(1 file, 1 obsolete file)
As part of rolling out ESLint across the tree, we should enable it for all of devtools/client/storage/test
.
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
devtools/client/storage/test/*.html
line and the lines below it that start with!devtools/client/storage/test/
. - Run eslint
./mach eslint --fix devtools/client/storage/test/
- 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/client/storage/test/
at the end (this avoids committing .eslintignore at this stage)$ hg commit -m "Bug nnn - Enable ESLint for all of devtools/client/storage/test/ (automatic changes). r?Standard8" devtools/client/storage/test/
- For the remaining issues, you'll need to fix them by hand. To find them, run
./mach eslint devtools/client/storage/test/
.- no-unused-vars: These generally fall into one of two cases:
const foo = blah()
- if it is like this, it is generally alright to change toblah()
. Although thefoo
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 blah */
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.
- no-unused-vars: These generally fall into one of two cases:
- 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/storage/test/ (manual changes). r?Standard8"
- Post the two commits via phabricator. Please use moz-phab to submit them.
- In .eslintignore, remove the
- Please note:
- 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.
Assignee | ||
Comment 1•5 years ago
|
||
Hi Mark,
Could I get this bug?
I will build Firefox today and give it a try.
Thanks!
Flags: needinfo?(standard8)
Reporter | ||
Comment 2•5 years ago
|
||
Hi Mellina, yes sure. I've assigned it to you.
Assignee: nobody → yonashiro.mellina
Status: NEW → ASSIGNED
Flags: needinfo?(standard8)
Assignee | ||
Comment 3•5 years ago
|
||
Assignee | ||
Comment 4•5 years ago
|
||
Updated•5 years ago
|
Attachment #9097141 -
Attachment is obsolete: true
Updated•5 years ago
|
Attachment #9097485 -
Attachment description: Bug 1582658 - Enable ESLint for all of devtools/client/storage/test/ (automatic changes). r?Standard8 → Bug 1582658 - Enable ESLint for all of devtools/client/storage/test/. r?Standard8
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7ac0a413ce8e Enable ESLint for all of devtools/client/storage/test/. r=miker
Comment 6•5 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox71:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
You need to log in
before you can comment on or make changes to this bug.
Description
•