Remove sync metadata migration from passwords engine
Categories
(Firefox :: Sync, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox90 | --- | fixed |
People
(Reporter: markh, Assigned: bdk)
References
Details
Attachments
(1 file)
In bug 1377100 we added a little code to migrate metadata from prefs to being maintained by the logins storage manager. We should rip that out in a few months. Probably a good first bug, but I'll mark it as that when the time is right to actually do it.
|
Assignee | |
Comment 1•3 years ago
|
||
I think I have this close to implemented. Here's the changeset: https://hg.mozilla.org/try/rev/2936e733a1fdb9933e01b43bb6f61b0b25ff6a29. Couple questions on this one:
- Did I get this docstring correct? https://hg.mozilla.org/try/rev/2936e733a1fdb9933e01b43bb6f61b0b25ff6a29#l4.12
- Should I have added any tests? Are there already tests for the sync process?
I ran mach try. By accident, I didn't run the chooser, so I think I ran all the tests. Here are the results: https://treeherder.mozilla.org/jobs?repo=try&revision=968cd3648f46ac7158d26443ce03470d812db888. There was 1 possible regression, I'm not sure if that was because of my changes or just a testing issue.
|
|
Assignee | |
Comment 2•3 years ago
|
||
- Moved ensureCurrentSyncID logic to the LoginManager component
|
Reporter | |
Comment 3•3 years ago
|
||
(In reply to Ben Dean-Kawamura from comment #1)
- Did I get this docstring correct? https://hg.mozilla.org/try/rev/2936e733a1fdb9933e01b43bb6f61b0b25ff6a29#l4.12
LGTM.
- Should I have added any tests? Are there already tests for the sync process?
I'd expect existing tests to capture this and it would be unreasonable to ask for new tests here if they didn't because you really aren't changing any behaviour. However, you should be sure to do a manual test here - in a test profile connect to sync and ensure passwords sync correctly.
I ran
mach try. By accident, I didn't run the chooser, so I think I ran all the tests. Here are the results: https://treeherder.mozilla.org/jobs?repo=try&revision=968cd3648f46ac7158d26443ce03470d812db888. There was 1 possible regression, I'm not sure if that was because of my changes or just a testing issue.
That looks completely unrelated. However, if you are unsure, there is UI to retrigger failing tests, so typically I would just retrigger it once or twice, and it will either go green or fail with a different error.
|
|
Assignee | |
Comment 4•3 years ago
|
||
- Just tested a manual sync using
mach buildand it was successful. - Looks like tests pass when I re-ran them: https://treeherder.mozilla.org/jobs?repo=try&revision=968cd3648f46ac7158d26443ce03470d812db888
Seems like this one might be ready to merge, what's the next step?
|
|
Reporter | |
Updated•3 years ago
|
|
||
Updated•3 years ago
|
Pushed by mhammond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/076335ab7d08 Removing legacy code to fetch syncId from the preferences. r=markh
|
||
Comment 6•3 years ago
|
||
| bugherder | ||
Description
•