Enable ESLint for uriloader
Categories
(Firefox :: File Handling, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: standard8, Assigned: aqadri, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=js])
Attachments
(2 files, 1 obsolete file)
As part of rolling out ESLint across the tree, we should enable it for uriloader/.
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
uriloader/...
lines. - Run eslint
./mach eslint --fix uriloader
- 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
uriloader
at the end (this avoids committing .eslintignore at this stage)$ hg commit -m "Bug nnn - Enable ESLint for uriloader (automatic changes). r?Standard8" uriloader
- For the remaining issues, you'll need to fix them by hand. To find them, run
./mach eslint uriloader
. - 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 uriloader (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•6 years ago
|
||
Hi Mark, I am an Outreachy applicant. Can I take this issue?
Reporter | ||
Comment 2•6 years ago
|
||
Hi @aqadri, sure, looks like you've almost finished your existing bug, so now's a good time to start a new one :-)
Assignee | ||
Comment 3•6 years ago
|
||
Updated•6 years ago
|
Assignee | ||
Comment 4•6 years ago
|
||
Assignee | ||
Comment 5•6 years ago
|
||
Reporter | ||
Comment 6•6 years ago
|
||
:aqadri, any idea why there's another changeset of manual changes now? Did you accidentally create an additional changeset? Maybe check the output of hg wip
.
You can ask in #introduction on IRC if you need realtime help.
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Comment 8•6 years ago
|
||
Backed out 2 changesets (bug 1532941) for mochitest failures in uriloader/exthandler/tests/mochitest/test_handlerApps.xhtml
Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=233833618&repo=autoland&lineNumber=4983
Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bf03fb29b472b6a1bbf7026877a6473f3502fea2
Backout:
https://hg.mozilla.org/integration/autoland/rev/14c61ee043c8e0507c05e13b8c82a0610df660fa
Reporter | ||
Comment 9•6 years ago
|
||
:aqadri - I think to fix the test issues, you'll need to undo the manual changes to handlerApps.js and add insert a line under the header (with a blank line above and below):
/* eslint-disable mozilla/use-services */
When you update the patches, you'll need to go to the action menu and re-open both of the phabricator revisions. Then re-push both of them - otherwise we won't be able to re-land them.
Assignee | ||
Updated•6 years ago
|
Reporter | ||
Comment 10•6 years ago
|
||
Thanks for the update, try server looks good now, so I'll re-push this.
Comment 11•6 years ago
|
||
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9e322c3c5db7
https://hg.mozilla.org/mozilla-central/rev/c1dfac003eb7
Description
•