Closed Bug 1314918 Opened 3 years ago Closed 3 years ago

Fix most of the remaining no-unused-vars issues for local scope in browser/

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file)

This is the next step for bug 1312407. This fixes most of the no-unused-vars for local scope in browser/.

There's only a handful of issues left, but they either need other patches to land, or some investigation for fixing them.

So in the meantime, lets get this patch landed to clean up (there's a few fixes in here as well).
Comment on attachment 8807116 [details]
Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

https://reviewboard.mozilla.org/r/90394/#review90126

::: browser/components/contextualidentity/test/browser/browser_forgetaboutsite.js:184
(Diff revision 1)
>  
>  
>    // Check that caches have been set correctly.
>    for (let userContextId of Object.keys(USER_CONTEXTS)) {
> -    let mem = getCacheStorage("memory");
> -    let disk = getCacheStorage("disk");
> +    let mem = getCacheStorage("memory", LoadContextInfo.custom(false, {userContextId}));
> +    let disk = getCacheStorage("disk", LoadContextInfo.custom(false, {userContextId}));

Why is this necessary?

::: browser/components/feeds/test/bug494328-data.xml:9
(Diff revision 1)
>      <title>Channel title</title>
>      <description>Channel description</description>
>      <link>Channel link</link>
>      <item>
>        <title>Episode 1</title>
> -      <enclosure url="http://www.example.com/podcasts/Episode%201" length="0" type="audio/x-m4a" />
> +      <enclosure url="http://www.example.com/podcasts/Episode%201" length="12340" type="audio/x-m4a" />

Did eslint catch this?

::: browser/components/places/content/editBookmarkOverlay.js:199
(Diff revision 1)
>      // trying to init it again, or we could end up leaking due to observers.
>      if (this.initialized)
>        this.uninitPanel(false);
>  
> -    let { itemId, itemGuid, isItem,
> -          isURI, uri, title,
> +    let { itemId, itemGuid, isItem, // eslint-disable-line no-unused-vars
> +          isURI, uri, title, // eslint-disable-line no-unused-vars

Why disable rather than omit?

::: browser/components/places/content/places.js:1372
(Diff revision 1)
>     */
>    get currentViewOptions() {
>      // Use ContentTree options as default.
>      let viewOptions = ContentTree.viewOptions;
>      if (this._specialViews.has(this.currentPlace)) {
> -      let { view, options } = this._specialViews.get(this.currentPlace);
> +      let { view, options } = this._specialViews.get(this.currentPlace); // eslint-disable-line no-unused-vars

Same question here

::: browser/components/translation/test/browser_translation_telemetry.js:222
(Diff revision 1)
>    gBrowser.removeTab(tab);
>  });
>  
>  add_task(function* test_language_change() {
> -  for (let i of Array(4)) {
> +  // This is run 4 times, the total additions are checked afterwards.
> +  for (let i of Array(4)) { // eslint-disable-line no-unused-vars

Huh. Clever I guess
Attachment #8807116 - Flags: review?(dtownsend)
Comment on attachment 8807116 [details]
Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

https://reviewboard.mozilla.org/r/90394/#review90126

> Why is this necessary?

The test looked like it was designed to iterate over `USER_CONTEXTS`, however `userContextId` was never used. The previous for loop did use the userContextId to open/add the cache entries. This loop seemed to be testing the additions, but had missed the actually checking the individual `userContextID`.

> Did eslint catch this?

Nope, I was testing something and forgot to undo it.

> Why disable rather than omit?

I was forgetting how object assignments work in an es6 world.
Comment on attachment 8807116 [details]
Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

https://reviewboard.mozilla.org/r/90394/#review90252
Attachment #8807116 - Flags: review?(dtownsend) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9601d282b06b
Fix most of the remaining no-unused-vars issues for local scope in browser/. r=mossop
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/581e573c82e6
Revert browser eslint rule changes to unbust eslint task; r=Standard8
https://hg.mozilla.org/mozilla-central/rev/9601d282b06b
https://hg.mozilla.org/mozilla-central/rev/581e573c82e6
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.