Closed Bug 1312277 Opened 4 years ago Closed 4 years ago

Turn on no-unused-vars for browser/experiments

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file)

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.
Blocks: 1312407
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
Attachment #8803712 - Flags: review?(dtownsend) → review+
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.
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2eca685ff243
Enable no-unused-vars eslint rule for browser/experiments. r=mossop
https://hg.mozilla.org/mozilla-central/rev/2eca685ff243
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.