Closed
Bug 1309632
Opened 8 years ago
Closed 7 years ago
Review services/fxaccounts/*.jsm for possible dead code to remove
Categories
(Firefox :: Sync, defect, P3)
Firefox
Sync
Tracking
()
RESOLVED
DUPLICATE
of bug 1448165
People
(Reporter: mds, Assigned: 17039, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug)
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
Details |
During the removal of dom/identity (and mozId) it has been noticed that services/fxaccounts/FxAccountsManager.jsm might be partially (or entirely) obsolete.
As suggested on https://bugzilla.mozilla.org/show_bug.cgi?id=1309030#c10 a review of services/fxaccounts/*.jsm may be needed to assess whether that code is still needed or not.
Updated•8 years ago
|
Priority: -- → P2
Updated•8 years ago
|
Priority: P2 → P1
Comment 2•8 years ago
|
||
It might be good to wait to do this until after the Sync tests are fixed.
Priority: P1 → P3
Comment 3•8 years ago
|
||
I think all we need to do now is remove `FxAccountsManager.jsm`, `ON_FXA_UPDATE_NOTIFICATION`, and maybe some tests. This is a great first bug.
Mentor: kit
Keywords: good-first-bug
Comment 4•7 years ago
|
||
I wish to work on this bug, hence assigning myself :)
Assignee: nobody → hossainalikram
Status: NEW → ASSIGNED
Comment 5•7 years ago
|
||
> I think all we need to do now is remove `FxAccountsManager.jsm`,
> `ON_FXA_UPDATE_NOTIFICATION`, and maybe some tests.
Is there any specific tests that you want to be removed?
Flags: needinfo?(kit)
Comment 6•7 years ago
|
||
Looks like `FxAccountsManager.jsm` was removed some time ago, so I think all that's left is removing `ON_FXA_UPDATE_NOTIFICATION`. \o/ I'm not sure if we have any that cover that; do you see any failures if you run `./mach test services/fxaccounts`? If not, feel free to just remove that notification, and comments that mention "FxAccountsManager". Thanks!
Flags: needinfo?(kit)
Comment hidden (mozreview-request) |
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8944568 [details]
Bug 1309632 - removed ON_FXA_UPDATE_NOTIFICATION line from services/fxaccounts/FxAccountsCommon.js;
https://reviewboard.mozilla.org/r/214734/#review220388
Thanks!
Attachment #8944568 -
Flags: review?(kit) → review+
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8944568 [details]
Bug 1309632 - removed ON_FXA_UPDATE_NOTIFICATION line from services/fxaccounts/FxAccountsCommon.js;
https://reviewboard.mozilla.org/r/214734/#review220392
Actually, please remove https://searchfox.org/mozilla-central/rev/b7e3ec2468d42fa59d86c03ec7afeb209813f1d4/services/fxaccounts/FxAccounts.jsm#1291-1292, and `"ON_FXA_UPDATE_NOTIFICATION"` from https://searchfox.org/mozilla-central/rev/b7e3ec2468d42fa59d86c03ec7afeb209813f1d4/tools/lint/eslint/modules.json#80.
Attachment #8944568 -
Flags: review+
Updated•7 years ago
|
Assignee: hossainalikram → 17039
Comment 10•7 years ago
|
||
ON_FXA_UPDATE_NOTIFICATION has been removed by bug 1448165, thanks!
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•