Closed
Bug 1556849
Opened 5 years ago
Closed 5 years ago
Enable ESLint for dom/base/*.*
Categories
(Core :: DOM: Core & HTML, task)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla70
People
(Reporter: standard8, Assigned: soniasingla, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=js])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
As part of rolling out ESLint across the tree, we should enable it for dom/base/*.*
.
To help Mozilla out with this bug, here's the steps:
- Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
- 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.
- Start working on this bug
- Please note:
- We want to end up with two separate commits. One with the automatic changes, the second with the manual changes.
- Although you'll change .eslintignore at the start, only the second commit should have the .eslintignore changes. Please follow the suggested commands closely.
- Here's what to do:
- In .eslintignore, remove the
dom/base/*.*
line. - Run eslint
./mach eslint --fix dom/base
- This should fix some of the issues.
- Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok.
- Create a commit of the work so far. Note the extra
dom/base
at the end (this avoids committing .eslintignore at this stage)$ hg commit -m "Bug nnn - Enable ESLint for dom/base (automatic changes). r?Standard8" dom/base
- For the remaining issues, you'll need to fix them by hand. To find them, run
./mach eslint dom/base
. - Create a second commit of the manual changes, note, there's no directory specifier this time, so .eslintignore will be included.
$ hg commit -m "Bug nnn - Enable ESLint for dom/base (manual changes). r?Standard8
- Post the two commits via phabricator. Please use moz-phab to submit them.
- In .eslintignore, remove the
- Please note:
- 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 dom peer, so there may still be more to go.
- 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!
- 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.
Assignee | ||
Comment 1•5 years ago
|
||
Hi Mark :)
I would like to work on this issue.
Thanks
Sonia
Comment 2•5 years ago
|
||
Hey! Can I take up this issue as my first contribution?
Thanks, Rishabh
Comment 3•5 years ago
|
||
(In reply to Sonia from comment #1)
Hi Mark :)
I would like to work on this issue.
Thanks
Sonia
Oh sorry, didn't see your comment there, go ahead!
Assignee | ||
Comment 4•5 years ago
|
||
(In reply to Rishabh Budhiraja from comment #3)
(In reply to Sonia from comment #1)
Hi Mark :)
I would like to work on this issue.
Thanks
SoniaOh sorry, didn't see your comment there, go ahead!
Thanks Rishabh :)
Reporter | ||
Comment 5•5 years ago
|
||
Hi Sonia, thank you for the offer, I've assigned it to you.
Assignee: nobody → soniasingla.1812
Assignee | ||
Comment 6•5 years ago
|
||
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cff2824766d7
Enable ESLint for dom/base/. r=Standard8,mccr8
Comment 8•5 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox70:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Reporter | ||
Comment 9•5 years ago
|
||
Thank you for working on this Sonia, its great to get it landed.
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•