1315951 - Enable no-unused-vars for toolkit/components

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Mark Banner (:standard8)]

I'm working on patches to enable no-unused-vars for toolkit/components. See bug 1315949 for more details.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1315951 - Enable no-unused-vars for toolkit/components/places.

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

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1315951 - Fix no-unused-vars issues in toolkit/components (except places).

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

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 8808969 [details]
Bug 1315951 - Fix no-unused-vars issues in toolkit/components (except places).

https://reviewboard.mozilla.org/r/91670/#review91616

::: toolkit/components/crashes/CrashService.js:65
(Diff revision 1)
>  
>    observe: function (subject, topic, data) {
>      switch (topic) {
>        case "profile-after-change":
>          // Side-effect is the singleton is instantiated.
> -        let m = Services.crashmanager;
> +        let m = Services.crashmanager; // eslint-disable-line no-unused-vars

You don't need the variable here for the side effect to happen

::: toolkit/components/jsdownloads/test/unit/head.js:826
(Diff revision 1)
>    }));
>  
>    // Get a reference to nsIComponentRegistrar, and ensure that is is freed
>    // before the XPCOM shutdown.
> -  let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
> +  let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar); // eslint-disable-line no-unused-vars
>    do_register_cleanup(() => registrar = null);

This doesn't look like it does anything to me. Do you disagree?

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8808968 [details]
Bug 1315951 - Enable no-unused-vars for toolkit/components/places.

https://reviewboard.mozilla.org/r/91668/#review91626

Thank you, this looks great!

::: toolkit/components/places/tests/browser/browser_favicon_setAndFetchFaviconForPage_failures.js:44
(Diff revision 1)
>  
>    function checkFavIconsDBCount(aCallback) {
>      let stmt = DBConn().createAsyncStatement("SELECT url FROM moz_favicons");
>      stmt.executeAsync({
>        handleResult: function final_handleResult(aResultSet) {
> -        for (let row; (row = aResultSet.getNextRow()); ) {
> +        for (let row; (row = aResultSet.getNextRow()); ) { // eslint-disable-line no-unused-vars

can we use a "while (aResultSet.getNextRow())" instead?

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8808968 [details]
Bug 1315951 - Enable no-unused-vars for toolkit/components/places.

https://reviewboard.mozilla.org/r/91668/#review91626

> can we use a "while (aResultSet.getNextRow())" instead?

Yes, that's much nicer.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8808969 [details]
Bug 1315951 - Fix no-unused-vars issues in toolkit/components (except places).

https://reviewboard.mozilla.org/r/91670/#review91616

> You don't need the variable here for the side effect to happen

Thanks, I've tidied it up.

> This doesn't look like it does anything to me. Do you disagree?

Further checking shows it isn't required at all - we don't even need to get the registrar, hence I've got rid of those lines.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 8808968 [details]
Bug 1315951 - Enable no-unused-vars for toolkit/components/places.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91668/diff/1-2/

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 8808969 [details]
Bug 1315951 - Fix no-unused-vars issues in toolkit/components (except places).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91670/diff/1-2/

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 8808969 [details]
Bug 1315951 - Fix no-unused-vars issues in toolkit/components (except places).

https://reviewboard.mozilla.org/r/91670/#review92044

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8808968 [details]
Bug 1315951 - Enable no-unused-vars for toolkit/components/places.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91668/diff/2-3/

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 8808969 [details]
Bug 1315951 - Fix no-unused-vars issues in toolkit/components (except places).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91670/diff/2-3/

Comment 12

[Mark Banner (:standard8)]

That was a minor update for bitrot.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e9417d75175
Enable no-unused-vars for toolkit/components/places. r=mak
https://hg.mozilla.org/integration/autoland/rev/6868b4b3f1ae
Fix no-unused-vars issues in toolkit/components (except places). r=mossop

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8e9417d75175
https://hg.mozilla.org/mozilla-central/rev/6868b4b3f1ae