Closed
Bug 1354039
Opened 7 years ago
Closed 7 years ago
Remove unused imports from Telemetry
Categories
(Toolkit :: Telemetry, enhancement, P3)
Toolkit
Telemetry
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)
1.66 KB,
patch
|
gfritzsche
:
review+
|
Details | Diff | Splinter Review |
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?
Reporter | ||
Comment 3•7 years ago
|
||
(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.
Reporter | ||
Updated•7 years ago
|
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).
Reporter | ||
Comment 6•7 years ago
|
||
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+
Reporter | ||
Comment 7•7 years ago
|
||
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
Comment 9•7 years ago
|
||
bugherder |
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.
Description
•