Closed
Bug 1354519
Opened 7 years ago
Closed 7 years ago
De-duplicate already ESLint recommended rules in toolkit/components/passwordmgr/.eslintrc.js
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement)
Tracking
(firefox55 fixed)
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: standard8, Assigned: hemantsingh1612, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=js])
Attachments
(1 file)
We now have a recommend set of rules for ESLint in Mozilla, I'd like for us to tidy up duplicates, so that it is easier to see where modules/different areas are different. This bug is to tidy up toolkit/components/passwordmgr/.eslintrc.js I'm happy to mentor this. Recommended rules: https://dxr.mozilla.org/mozilla-central/source/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js What needs doing (all in the file toolkit/components/passwordmgr/.eslintrc.js): - Remove the extends section, as this is unnecessary (ESLint inherits rules through the directory tree). - For the rules, globals and env sections, go through each one and compare to the recommended, then: -- If it is the same (with the same parameters), remove the rule -- If it is commented, remove it -- If it is different, leave it in. - Finally check ESLint still runs correctly (./mach eslint) and passes with no errors (warnings are ok if they are the same as previous). Once that's done, attach a patch and request review from me (Standard8)
Assignee | ||
Comment 1•7 years ago
|
||
Hey I want to work on this bug.Can someone assign it to me please?
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → hemantsingh1612
Reporter | ||
Comment 2•7 years ago
|
||
I've just realised there's extra cleanup that can be done as well: - If the .eslintrc.js file lists a rule as 'warn' and recommended.js lists it as 'error', then drop the rule in the .eslintrc.js file. This will promote the rule to "error" which is better as it'll get fixed if it is an error, where as warnings tend to get ignored.
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•7 years ago
|
||
Thanks for info :).
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
Hi Mark Please have a look at the patch and direct me for changes(if needed).
Reporter | ||
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8861526 [details] Bug 1354519 - De-duplicate already ESLint recommended rules in toolkit/components/passwordmgr/.eslintrc.js. https://reviewboard.mozilla.org/r/133494/#review136712 Thank you for the patch. Unfortunately this will need rebasing onto the latest code as another bug has added in a "complexity" rule (that will need to stay in). Apart from that, just one minor issue to ensure ESLint keeps passing. ::: toolkit/components/passwordmgr/.eslintrc.js (Diff revision 1) > - > - // No using undeclared variables > - "no-undef": "error", > - > - // Don't allow unused local variables unless they match the pattern > - "no-unused-vars": ["error", {"args": "none", "vars": "local", "varsIgnorePattern": "^(ids|ignored|unused)$"}], `no-unused-vars` needs leaving in currently due to the different varsIgnorePattern.
Attachment #8861526 -
Flags: review?(standard8)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8861526 [details] Bug 1354519 - De-duplicate already ESLint recommended rules in toolkit/components/passwordmgr/.eslintrc.js. https://reviewboard.mozilla.org/r/133494/#review137008 Thank you for the update. Looks good. r=Standard8
Attachment #8861526 -
Flags: review?(standard8) → review+
Reporter | ||
Comment 9•7 years ago
|
||
I've triggered this to land, however it'll probably take a while as the trees are currently closed atm.
Comment 10•7 years ago
|
||
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5cdda2e95bd7 De-duplicate already ESLint recommended rules in toolkit/components/passwordmgr/.eslintrc.js. r=standard8
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5cdda2e95bd7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•6 years ago
|
Product: Testing → Firefox Build System
Updated•5 years ago
|
Keywords: good-first-bug
Version: Version 3 → 3 Branch
Updated•5 years ago
|
Keywords: good-first-bug
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•