Closed Bug 1439838 Opened 2 years ago Closed 2 years ago

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

Categories

(Firefox for Android :: General, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: standard8, Assigned: standard8)

Details

Attachments

(1 file)

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 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 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)
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
https://hg.mozilla.org/mozilla-central/rev/6f787e71c76f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.