Closed Bug 1564555 Opened 4 months ago Closed 4 months ago

Enable some more ESLint rules for layout/

Categories

(Core :: Layout, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: standard8, Assigned: ruchikag826, Mentored)

References

Details

Attachments

(1 file)

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).
  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.
    • 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.
  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.
Priority: -- → P3
Summary: Enable some more rules for layout/ → Enable some more ESLint rules for layout/

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?)

Flags: needinfo?(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.

Flags: needinfo?(standard8)

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

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

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cd8478a8a714
Enable some more ESLint rules for layout. r=Standard8,dholbert
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
See Also: → 1596160
You need to log in before you can comment on or make changes to this bug.