1656295 - Enable ESLint no-unused-vars rule for netwerk/cookie/test and netwerk/test

Status: RESOLVED FIXED

(Core :: Networking, task)

Description

[Mark Banner (:standard8)]

When we initially enabled ESLint for netwerk/, we disabled various rules to make it simpler.

This bug is to enable no-unused-vars for netwerk/cookie/test and netwerk/test.

https://searchfox.org/mozilla-central/rev/56bb74ea8e04bdac57c33cbe9b54d889b9262ade/.eslintrc.js#175

Comment 1

[Mark Banner (:standard8)]

To help Mozilla out with this bug, here's the steps:

1. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
2. Follow the guide for contributing: https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
   • If you have any problems, please ask on Matrix in the #introduction channel. They're there to help you get started.
3. Start working on this bug
   • Here's what to do:
     1. Remove the line in the top-level .eslintrc.js referenced in comment 0.
     2. Run eslint ./mach eslint netwerk/*
     3. Fix the issues raised, most of these will fall into the following categories:
        • A variable that is unused and has no side-effects, e.g. var foo = 1. This can just be removed.
        • A variable that is unused, but has side-effects, e.g. let profile = do_get_profile();. In this case, just remove the variable definition, e.g. do_get_profile();
        • A variable assignment to an object or array, e.g. let [foo, bar] = myfunc() where foo is unused. Just drop the unnecessary variable, e.g. let [, bar] = myfunc().
        • If you get stuck feel free to ask.
     4. Do a build and run the tests. For running the tests, I'd suggest:
        • ./mach mochitest netwerk/cookie/test/browser/ netwerk/test/browser/ netwerk/test/mochitests/
        • ./mach xpcshell-test netwerk/test/unit*/
     5. Create a commit of the manual changes:
        • $ hg commit -m "Bug nnn - Enable ESLint rule no-unused-vars for all of netwerk/. r?Standard8"
     6. Post the commits as per the contributing guide.
4. Once the patches are submitted, I'll take a look. If there's any changes necessary I'll comment in Phabricator, so be prepared to update the patches. If there's no changes, I'll request review from a netwerk peer, so there may still be more to go.
   • If you do need to update the commit, please amend the existing commit, rather than creating new ones. This helps with tracking of review comments.
5. Once we're happy with the changes, I'll push it to autoland - our integration branch. Your code will then soon be shipping to Firefox users worldwide!
6. Now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.

Comment 2

[atisheyjain1232]

hello, i want to work on it

Comment 3

[Mark Banner (:standard8)]

Hi, please do start working on it, I'll assign it to you. Feel free to ask questions. If you decide at some stage not to continue, that's fine, just please let us know so we can re-assign it.

Comment 4

[atisheyjain1232]

I don't want to continue

Comment 5

[Mark Banner (:standard8)]

No problem, thank you for letting us know.

Comment 6

[João Vanzuita]

hey Mark, I would like to work on this task. I actually just started.

Comment 7

[Mark Banner (:standard8)]

Hi João, that's great. Let us know if you have any questions.

Comment 8

[João Vanzuita]

Attachment: Bug 1656295 - Enable ESLint rule no-unused-vars for all of netwerk/.

Comment 9

[João Vanzuita]

hey Mark,

One test is not passing, and I'm not familiar with the test suit, could you have a look?

I just published the patch here https://phabricator.services.mozilla.com/D92333

Comment 10

[João Vanzuita]

Attachment: Bug 1656295 add code review changes request

Depends on D92333

Comment 11

[João Vanzuita]

Missing test was fixed, all tests are passing now.

Comment 12

[Mark Banner (:standard8)]

João, did you see my comments here: https://phabricator.services.mozilla.com/D92333#2996967

Would you be able to try and update the patches please?

Comment 13

[João Vanzuita]

Hey Mark, sorry for the delay.

The source control tool is something down my motivation for contributing. I'll follow your advice and ask for help in the channel.

Comment 14

[Vivek pandey]

I would like to work on this card as my first pr, please let me if needed

Comment 15

[Mark Banner (:standard8)]

(In reply to erpandey from comment #14)

I would like to work on this card as my first pr, please let me if needed

Hi, sorry this one is already in progress. There are a couple here that I have that haven't been taken yet: https://codetribute.mozilla.org/projects/ff?project%3DSearch

Comment 16

[Mark Banner (:standard8)]

Taking for now as I'm finishing this off.

Try push: https://treeherder.mozilla.org/jobs?repo=try&revision=1db5651332a986fbf957c8da90c6d181b18caccc&selectedTaskRun=XmwMAr5_QIiJCdFNsdOT7Q.0

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a738cbbd68a
Enable ESLint rule no-unused-vars for all of netwerk/. r=necko-reviewers,dragana

Comment 18

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/7a738cbbd68a

Comment 19

[Mark Banner (:standard8)]

Assigning this back to João as they did most of the work here - thank you for working on it, I'm sorry it turned out a bit bigger than expected.