Closed Bug 1532941 Opened 7 months ago Closed 6 months ago

Enable ESLint for uriloader

Categories

(Firefox :: File Handling, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: standard8, Assigned: aqadri, Mentored)

References

(Blocks 1 open bug)

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:

  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
    • 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:
      1. In .eslintignore, remove the uriloader/... lines.
      2. Run eslint ./mach eslint --fix uriloader
        • This should fix some of the issues.
      3. Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok.
      4. 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
      5. For the remaining issues, you'll need to fix them by hand. To find them, run ./mach eslint uriloader.
        • Most of those should be reasonably easy to understand, but there's more information on some specific bits (especially no-undef) here and here.
      6. 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
      7. Post the two 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 dom peer, so there may still be more to go.
  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.

Hi Mark, I am an Outreachy applicant. Can I take this issue?

Hi @aqadri, sure, looks like you've almost finished your existing bug, so now's a good time to start a new one :-)

Assignee: nobody → asra.qadri
Status: NEW → ASSIGNED
Priority: -- → P3
Blocks: 1533719

: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.

Flags: needinfo?(asra.qadri)
Flags: needinfo?(asra.qadri)
Attachment #9049734 - Attachment is obsolete: true
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c5190e74039c
Enable ESLint for uriloader (automatic changes). r=Standard8,qdot
https://hg.mozilla.org/integration/autoland/rev/bf03fb29b472
Enable ESLint for uriloader (manual changes). r=Standard8,qdot

: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.

Flags: needinfo?(asra.qadri)

Thanks for the update, try server looks good now, so I'll re-push this.

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9e322c3c5db7
Enable ESLint for uriloader (automatic changes). r=Standard8,qdot
https://hg.mozilla.org/integration/autoland/rev/c1dfac003eb7
Enable ESLint for uriloader (manual changes). r=Standard8,qdot
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.