1582666 - Enable more ESLint rules for netwerk/

Status: RESOLVED FIXED

(Core :: Networking, task, P3)

Description

[Mark Banner (:standard8)]

When we first enabled ESLint on netwerk/, we disabled various rules. This bug is for enabling some more of them, specifically:

• mozilla/consistent-if-bracing
• mozilla/reject-importGlobalProperties
• mozilla/use-default-preference-values
• no-array-constructor
• no-new-wrappers
• no-return-await
• no-sequences
• no-unreachable
• no-useless-return

(although this is a lot of rules, there's only a few failures of each one at most).

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. Download and build the Firefox source code, see the docs for details. An artifact build is all you need.
   • If you have any problems, please ask on IRC in the #introduction channel. They're there to help you get started.
   • You can also read the Developer Guide, which has answers to most development questions.
3. Start working on this bug
   • Here's what to do:
     1. Remove the rules in .eslintrc.js that we want to enable. See the link for details.
     2. Run eslint ./mach eslint netwerk/
     3. Fix the issues raised.
        • Most of the rules should be reasonably easy to understand, but there's more information on specific bits here and details of the rules here.
        • If you're not sure how to fix an issue, feel free to ask here or in #introduction on IRC
     4. Create a commit of the manual changes:
        • $ hg commit -m "Bug nnn - Enable more ESLint rules for netwerk/. r?Standard8"
     5. Post the commits via phabricator. Please use moz-phab to submit them.
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 devtools 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 1

[u647686]

Hi Mark, I'd like to help out on this bug.

Comment 2

[Mark Banner (:standard8)]

Hi Ryan, thank you for offering, I've assigned it to you. Please start working on it, and ask questions if you need to.

Comment 3

[u647686]

Thanks Mark, I've gotten started on it. I've built Firefox so I'm working through step 3 now.

Comment 4

[Mark Banner (:standard8)]

Hi Ryan, how are you getting on, do you need any help?

Comment 5

[u647686]

Hi Mark, my sincere apologies for the delay. I have made some progress and I do intend to finish this, but I've been swamped with schoolwork in the past month. I'm hoping to get this done by the end of next week.

Comment 6

[Mark Banner (:standard8)]

(In reply to Ryan Apilado from comment #5)

Hi Mark, my sincere apologies for the delay. I have made some progress and I do intend to finish this, but I've been swamped with schoolwork in the past month. I'm hoping to get this done by the end of next week.

No problem, schoolwork is important. I was just checking in.

Comment 7

[Mahak]

heya!!
Mark, I would love to work on this!!
Can I take this up ?
Thanks
Mahak :)

Comment 8

[Mark Banner (:standard8)]

Ok, I attempted to contact Ryan last week, but haven't had a response, so I can pass it to you Mahak.

Comment 9

[Mahak]

@Thanks for assigning this to me .
I'll submit the patch as soon as possible :)
Thanks

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Note that if we're doing this, it would be nice to include "use strict"; at the top of the JS test files.
There are still a bunch of them that don't have it.

Comment 11

[Mahak]

@valentin I think you are talking about these files https://searchfox.org/mozilla-central/rev/fa2df28a49883612bd7af4dacd80cdfedcccd2f6/.eslintrc.js#158-161
Right?
or some other files ?

Comment 12

[Mahak]

Attachment: Bug 1582666 - Enable more ESLint rules for netwerk/. r?standard8

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c9861998d1e
Enable more ESLint rules for netwerk/. r=Standard8,valentin

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3c9861998d1e