1169558 - [Messages][NG] Lay out Conversation service structure

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

Based on ConversationService spec from bug 1169543, we'd like to create basic ConversationService interface here.

Patch for this bug should contain full service/client definition without real method implementation (just stub methods throwing "Not Implemented" exception for now).

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: master github PR

Hey Oleg, Steve,

Please tell me what you think :)

I added a ServiceMixin that I stole both from Steve's work in mozMobileMessagingShim and Oleg's Event Dispatcher :)

Should I add unit tests for this too?

Comment 2

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8621003 [details] [review]
master github PR

Looks good to me! Added some nits and questions at GitHub.

Thanks!

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 8621003 [details] [review]
master github PR

I fixed the nits and added a unit test.

Tell me what you think !

Do we need to do the client right now ?

Comment 4

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8621003 [details] [review]
master github PR

(In reply to Julien Wajsberg [:julienw] from comment #3)
> Comment on attachment 8621003 [details] [review]
> master github PR
> 
> I fixed the nits and added a unit test.
> 
> Tell me what you think !

Looks good, just few tiny nits/questions at github!

> 
> Do we need to do the client right now ?

Honestly, I don't think so. IMO it's hard to say how client will look like right now.

Comment 5

[Julien Wajsberg [:julienw]]

I fixed everything Oleg reported; now waiting for Steve's review :)

Comment 6

[Julien Wajsberg [:julienw]]

master: 2157bb84cf217ecb3d217a2f1ee4282d59ea248c

Steve, if you notice anything, we can fix in a follow-up.

Thanks !

Comment 7

[Steve Chung [:steveck]]

Comment on attachment 8621003 [details] [review]
master github PR

Overall looks good, just some question(for the bridge mixin):
- Would you prefer that all the services should follow this pattern that leaving all the bridge setup and interface in this bridge mixin?
- If it's our pattern for services, will we plan to support the service in non-worker context?

Comment 8

[Julien Wajsberg [:julienw]]

(In reply to Steve Chung [:steveck] from comment #7)
> Comment on attachment 8621003 [details] [review]
> master github PR
> 
> Overall looks good, just some question(for the bridge mixin):
> - Would you prefer that all the services should follow this pattern that
> leaving all the bridge setup and interface in this bridge mixin?

If we can do that, yep, I think it would be nice. I followed up with your approach in bug 1171865 because I thought it was a good idea, and extended it to make it more generic.

> - If it's our pattern for services, will we plan to support the service in
> non-worker context?

I don't think so; I think that even for the non-splitted-view app, we can use the services-in-worker. Or what do you have in mind ?