Closed Bug 1315951 Opened 4 years ago Closed 4 years ago

Enable no-unused-vars for toolkit/components

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(2 files)

I'm working on patches to enable no-unused-vars for toolkit/components. See bug 1315949 for more details.
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?
Attachment #8808969 - Flags: review?(dtownsend)
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?
Attachment #8808968 - Flags: review?(mak77) → review+
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 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 on attachment 8808969 [details]
Bug 1315951 - Fix no-unused-vars issues in toolkit/components (except places).

https://reviewboard.mozilla.org/r/91670/#review92044
Attachment #8808969 - Flags: review?(dtownsend) → review+
That was a minor update for bitrot.
Pushed by mbanner@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/8e9417d75175
https://hg.mozilla.org/mozilla-central/rev/6868b4b3f1ae
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Depends on: 1319009
You need to log in before you can comment on or make changes to this bug.