Closed Bug 1180591 Opened 5 years ago Closed 5 years ago

[Messages][NG] mozSettings shim Implementation

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: steveck, Assigned: steveck)

References

Details

(Whiteboard: [sms-FxOS-S3])

Attachments

(1 file)

Based on the discussion in Bug 1179628, we decide to move settings into service. Considering the mozSettings could not work in worker, having a shim for mozSettings API is necessary in order to make settings service runnable.
Assignee: nobody → schung
Status: NEW → ASSIGNED
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

Small settings shim that only contains 3 methods :)
Attachment #8634660 - Flags: feedback?(felash)
Attachment #8634660 - Flags: feedback?(azasypkin)
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

good for me except I don't think we need addObserver, at least for now.
Attachment #8634660 - Flags: feedback?(felash) → feedback+
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

LGTM, left few minor nits at GitHub.
Attachment #8634660 - Flags: feedback?(azasypkin) → feedback+
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

Test added for review. No hurry since you are working on other critical tasks :)
Attachment #8634660 - Flags: review?(felash)
Attachment #8634660 - Flags: review?(azasypkin)
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

Left some comments on github.

Thanks !
Attachment #8634660 - Flags: review?(felash)
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

Looks good to me once Treeherder is green (it's still running)!

Thanks!
Attachment #8634660 - Flags: review?(azasypkin) → review+
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

I added console error for the settings not given case, but I didn't add db check for calling API part. Please tell me if you still think we should check before calling API, thanks!
Attachment #8634660 - Flags: review?(felash)
Comment on attachment 8634660 [details] [review]
[gaia] steveck-chung:new-message-settings-shim > mozilla-b2g:master

I think it's good to go; just one nit and one question but r=me with the nit and we can discuss about the question on IRC and figure this in a later patch.
Attachment #8634660 - Flags: review?(felash) → review+
Thanks and nit addressed! If the question is about where to init the shims, I think Oleg's shim bootstrap is reasonable(actually I can't think of other better way than this).

In master: https://github.com/mozilla-b2g/gaia/commit/da7be86023eed90054b6d12cf884abb1caf247db
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [sms-FxOS-S3]
You need to log in before you can comment on or make changes to this bug.