Eliminate Places dependency in Sync utils

RESOLVED FIXED in mozilla31

Status

Cloud Services
Firefox Sync: Backend
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: tatiana, Assigned: rnewman)

Tracking

unspecified
mozilla31
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8400394 [details] [diff] [review]
Allow to use services/sync without MOZ_PLACES defined

It would be nice to be able include and use some of services/sync components without dependency on MOZ_PLACES.
Attachment #8400394 - Flags: review?(mhammond)
Does this actually allow sync to be used?  It seems both services/sync/modules/engines/bookmarks.js and services/sync/modules/engines/history.js need PlacesUtils and would fail without it?
(Assignee)

Comment 2

3 years ago
Comment on attachment 8400394 [details] [diff] [review]
Allow to use services/sync without MOZ_PLACES defined

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

I'll upload a better fix in just a moment.
Attachment #8400394 - Flags: review?(mhammond) → review-
(Assignee)

Comment 3

3 years ago
Created attachment 8400406 [details] [diff] [review]
Eliminate Places dependency in Sync utils. r=

This simply moves that code closer to the only place it's used, and eliminates the (largely pointless) test. This makes Weave.Utils independent of Places.
(Assignee)

Updated

3 years ago
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
OS: Linux → All
Hardware: x86_64 → All
Summary: Allow to use base of Sync services without PLACES dependency → Eliminate Places dependency in Sync utils
(Assignee)

Comment 4

3 years ago
Comment on attachment 8400406 [details] [diff] [review]
Eliminate Places dependency in Sync utils. r=

Services peer for review.
Attachment #8400406 - Flags: review?(gps)
(Reporter)

Updated

3 years ago
Depends on: 746800

Comment 5

3 years ago
Comment on attachment 8400406 [details] [diff] [review]
Eliminate Places dependency in Sync utils. r=

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

I also would have accepted a patch that moved this method into PlacesUtils.jsm itself :)
Attachment #8400406 - Flags: review?(gps) → review+
(Assignee)

Comment 6

3 years ago
Pushed with tests fixed to match:

https://hg.mozilla.org/integration/fx-team/rev/94fa2e17d418

Tatiana, I don't understand what relationship this has with Bug 746800, but given that I just landed the patch for this, I'll assume that it doesn't actually depend on that bug.
No longer depends on: 746800
https://hg.mozilla.org/mozilla-central/rev/94fa2e17d418
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
(Reporter)

Comment 8

3 years ago
Thanks a lot!
You need to log in before you can comment on or make changes to this bug.