1564555 - Enable some more ESLint rules for layout/

Status: RESOLVED FIXED

(Core :: Layout, task, P3)

Description

[Mark Banner (:standard8)]

In bug 1554224 we enabled ESLint for layout/, however we also disabled various rules. Now that bug has landed, we should work to enable the rules that were initially disabled.

For this first bug, we'll focus on rules that can be enabled automatically:

• mozilla/no-useless-parameters
• mozilla/no-useless-run-test
• mozilla/use-chromeutils-import
• dot-notation
• no-else-return

Ruchika has already agreed to take this bug on.

1. Download and build the Firefox source code: https://developer.mozilla.org/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build (an artifact build is sufficient).
   • If you have any problems, please ask on IRC (https://wiki.mozilla.org/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: https://developer.mozilla.org/docs/Mozilla/Developer_guide/Introduction
2. Start working on this bug.
   • Remove the relevant lines from the top-level .eslintrc.js
   • Run ./mach eslint layout --fix
   • If it doesn't fix everything, you'll need to investigate the issues raised and fix them.
     • The ESLint website has more details for most of the rules
     • The mozilla/ rules are documented separately
   • Please also run hg diff and check the output for any indentation that needs fixing (the html files especially around any else changes).
   • If you have direct questions about this bug, feel free to ask here or on irc.
3. Build your change with mach build and test your change with:
   • ./mach xpcshell-test layout
   • ./mach mochitest layout
4. Submit the patch for review. Mark me as a reviewer (r?standard8) so I'll get an email to come look at your code. I'll be doing an initial review of the patch, then passing it to layout/ peers for final review.
   • Here's the guide.
   • I strongly suggest using moz_phab to push the patch.
5. After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will 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

[Ruchika Gosain]

Attachment: Bug 1564555 - Enable some more ESLint rules for layout. r=standard8

Comment 2

[Daniel Holbert [:dholbert]]

setting needinfo=Standard8 to determine next action here, now that it's r+

(I think this can land? Or does it need a Try run or anything?)

Comment 3

[Mark Banner (:standard8)]

I ran the earlier patch through try and it was fine. Seeing as the only intermediate changes are reverting, I'll trigger this for landing.

Thank you Ruchika.

Comment 4

[Daniel Holbert [:dholbert]]

Thanks! Sorry to bug you late in your timezone, and enjoy your upcoming AFK time. :)

Comment 5

[Ruchika Gosain]

Welcome Daniel and Mark. Thankyou for your time and patience! Looking to learn more and contribute to Mozilla Central :)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd8478a8a714
Enable some more ESLint rules for layout. r=Standard8,dholbert

Comment 7

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/cd8478a8a714