Enable ESLint rule no-unused-vars for the same directories in mobile/android as no-undef

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
Firefox 60
Points:
---

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

no-unused-vars tends to work quite well in conjunction with no-undef, as they find a similar style of issue.

As we've already started enabling no-undef on mobile/android, I think we should also do no-unused-vars on the same directories. As we then progress through enabling no-undef in more places, we can also enable no-unused-vars at the same time.

Comment 2

a year ago
mozreview-review
Comment on attachment 8952649 [details]
Bug 1439838 - Enable ESLint rule no-unused-vars for the same directories in mobile/android as no-undef.

https://reviewboard.mozilla.org/r/221898/#review227774

Note: I just looked at the session store changes so far.

::: mobile/android/components/SessionStore.js:185
(Diff revision 1)
>        if (this._sessionFileTemp.exists()) { this._sessionFileTemp.remove(false); }
>      }
>    },
>  
>    _forgetClosedTabs: function ss_forgetClosedTabs() {
> -    for (let [ssid, win] of Object.entries(this._windows)) {
> +    for (let [/* ssid */, win] of Object.entries(this._windows)) {

If you're touching this anyway and don't mind manually adjusting the patch, you could also replace this by `let win of Object.values(this._windows)'
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8952649 [details]
Bug 1439838 - Enable ESLint rule no-unused-vars for the same directories in mobile/android as no-undef.

https://reviewboard.mozilla.org/r/221898/#review227922

Looks fine, but we still want to hang onto those debug helper functions in Geckoview for now.

::: mobile/android/modules/geckoview/GeckoViewContentModule.jsm
(Diff revision 2)
>  XPCOMUtils.defineLazyGetter(this, "dump", () =>
>      ChromeUtils.import("resource://gre/modules/AndroidLog.jsm",
>                         {}).AndroidLog.d.bind(null, "ViewContentModule"));
>  
> -function debug(aMsg) {
> -  // dump(aMsg);

Please don't remove this, it's still needed during development for the time being.

::: mobile/android/modules/geckoview/GeckoViewModule.jsm
(Diff revision 2)
>  XPCOMUtils.defineLazyGetter(this, "dump", () =>
>      ChromeUtils.import("resource://gre/modules/AndroidLog.jsm",
>                         {}).AndroidLog.d.bind(null, "ViewModule"));
>  
> -function debug(aMsg) {
> -  // dump(aMsg);

Ditto.

::: mobile/android/modules/geckoview/GeckoViewTab.jsm
(Diff revision 2)
>  XPCOMUtils.defineLazyGetter(this, "dump", () =>
>      ChromeUtils.import("resource://gre/modules/AndroidLog.jsm",
>                         {}).AndroidLog.d.bind(null, "ViewTab"));
>  
> -function debug(aMsg) {
> -  // dump(aMsg);

Ditto.
Attachment #8952649 - Flags: review?(jh+bugzilla) → review+
How about we just comment out the whole debug function rather than just the dump (since to use them you have to uncomment something)? When they are needed they can be un-commented...
Flags: needinfo?(jh+bugzilla)
Yeah, since you have to touch them anyway I guess that would be fine.
Flags: needinfo?(jh+bugzilla)
Comment hidden (mozreview-request)

Comment 8

a year ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f787e71c76f
Enable ESLint rule no-unused-vars for the same directories in mobile/android as no-undef. r=JanH

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6f787e71c76f
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.