1312277 - Turn on no-unused-vars for browser/experiments

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mark Banner (:standard8)]

I'm looking to turn on no-unused-vars throughout the tree as it is useful for ensuring that dead code isn't left lying around.

The place I found to start is browser/experiments. Patch coming up in a few.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1312277 - Enable no-unused-vars eslint rule for browser/experiments.

Review commit: https://reviewboard.mozilla.org/r/87844/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87844/

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 8803712 [details]
Bug 1312277 - Enable no-unused-vars eslint rule for browser/experiments.

https://reviewboard.mozilla.org/r/87844/#review87026

Looks good assuming tests are passing.

::: browser/experiments/Experiments.jsm:2222
(Diff revision 1)
>        }
>  
>        for (let id of removed) {
>          this._log.trace("updateExperimentList() - removing " + id);
>          let wrapper = new PreviousExperimentAddon(oldMap.get(id));
> -        AddonManagerPrivate.callAddonListeners("onUninstalling", plugin, false);
> +        AddonManagerPrivate.callAddonListeners("onUninstalling", wrapper, false);

I love it when eslint catches bugs!

::: browser/experiments/test/xpcshell/head.js:11
(Diff revision 1)
> +            EXPERIMENT1_NAME, EXPERIMENT1_XPI_SHA1, EXPERIMENT1A_NAME,
> +            EXPERIMENT1A_XPI_SHA1, EXPERIMENT2_ID, EXPERIMENT2_XPI_SHA1,
> +            EXPERIMENT3_ID, EXPERIMENT4_ID, FAKE_EXPERIMENTS_1,
> +            FAKE_EXPERIMENTS_2, gAppInfo, removeCacheFile, defineNow,
> +            futureDate, dateToSeconds, loadAddonManager, promiseRestartManager,
> +            startAddonManagerOnly, getExperimentAddons, replaceExperiments */

I wonder if it might be better to just turn off the rule for head files

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8803712 [details]
Bug 1312277 - Enable no-unused-vars eslint rule for browser/experiments.

https://reviewboard.mozilla.org/r/87844/#review87026

> I wonder if it might be better to just turn off the rule for head files

I think you might be right, although I think this is probably a more extreme example of a typical head.js. I'll bear it in mind when we look at enabling in other directories & we might revisit it.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2eca685ff243
Enable no-unused-vars eslint rule for browser/experiments. r=mossop

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2eca685ff243