Closed Bug 1354039 Opened 7 years ago Closed 7 years ago

Remove unused imports from Telemetry

Categories

(Toolkit :: Telemetry, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: gfritzsche, Assigned: purfling, Mentored)

References

Details

(Whiteboard: [measurement:client] [lang=js] [good first bug])

Attachments

(1 file)

PromiseUtils.jsm is unused in:
- TelemetryController.jsm
- TelmetryEnvironment.jsm

We should remove that import.

To make sure changes went fine:
- build: mach build
- run eslint: mach eslint
- run tests: mach test toolkit/components/telemetry/unit
Hello, I'm very new to Bugzilla and would like to get involved. This looked like a good bug to get started with if no one's currently working on it. I've removed the import from both TelemetryController.jsm and TelemetryEnvironment.jsm, built, and am running tests now.
eslint complained about 2 warnings: (I don't know if these have been reported elsewhere, I haven't checked yet)

/mozilla-central/browser/components/migration/MSMigrationUtils.jsm
540:40  warning  'aFile' is already declared in the upper scope.  no-shadow (eslint)

/mozilla-central/browser/extensions/formautofill/content/FormAutofillFrameScript.js
52:18  warning  Parameter 'aMessage' uses Hungarian Notation, consider using 'message' instead.  mozilla/no-aArgs (eslint)

All 28 unit tests passed:

 1:18.26 LOG: MainThread INFO INFO | Result summary:
 1:18.26 LOG: MainThread INFO INFO | Passed: 28
 1:18.26 LOG: MainThread INFO INFO | Failed: 0
 1:18.26 LOG: MainThread INFO INFO | Todo: 0
 1:18.26 LOG: MainThread INFO INFO | Retried: 0
 1:18.26 SUITE_END: MainThread 

Summary
=======

Ran 28 tests
Expected results: 28
Unexpected results: 0

Is it typical to attach full output from tests to the bug report?
(In reply to Matthew from comment #2)
> eslint complained about 2 warnings: (I don't know if these have been
> reported elsewhere, I haven't checked yet)
> 
> /mozilla-central/browser/components/migration/MSMigrationUtils.jsm
> 540:40  warning  'aFile' is already declared in the upper scope.  no-shadow
> (eslint)
> 
> /mozilla-central/browser/extensions/formautofill/content/
> FormAutofillFrameScript.js
> 52:18  warning  Parameter 'aMessage' uses Hungarian Notation, consider using
> 'message' instead.  mozilla/no-aArgs (eslint)

Hi Matthew!
These are outside the changed code, so that is fine.
I think a better eslint command is: mach eslint toolkit/components/telemetry
That should pass without errors.
Assignee: nobody → purfling
Please let me know if I should have split this patch into two separate patches (one for each of the modified files).
Comment on attachment 8855686 [details] [diff] [review]
Patch for bug 1354039

Review of attachment 8855686 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine, thanks!
Attachment #8855686 - Flags: review+
I already tested this locally, we don't need a try run for this.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/39021f2167a7
Remove unused PromiseUtils.jsm import from TelemetryController.jsm and TelemetryEnvironment.jsm. r=gfritzsche
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/39021f2167a7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.