Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(305)

Issue 4960060: Implement org.freedesktop.DBus.StartServiceByName. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 6 months ago by Daiki Ueno
Modified:
12 years, 6 months ago
Reviewers:
shawn.p.huang, Peng
Base URL:
git@github.com:ibus/ibus.git@master
Visibility:
Public.

Description

Implement org.freedesktop.DBus.StartServiceByName. BUG=none TEST=tested with https://github.com/ueno/ibus-gucharmap/tree/charmap-service

Patch Set 1 #

Total comments: 4

Patch Set 2 : don't connect to self signal #

Patch Set 3 : remove BusDBusImpl::start-service signal #

Total comments: 8

Patch Set 4 : add timeout for starting component #

Total comments: 4

Patch Set 5 : define IBUS_BUS_START_REPLY_* in ibustypes.h #

Total comments: 20

Patch Set 6 : return D-Bus error rather than replying 0, improve start_service_calls traversal #

Patch Set 7 : use GList instead of GSList for start_service_calls #

Patch Set 8 : support StartServiceByName in python binding #

Unified diffs Side-by-side diffs Delta from patch set Stats (+182 lines, -1 line) Patch
M bus/dbusimpl.c View 1 2 3 4 5 6 10 chunks +167 lines, -1 line 0 comments Download
M ibus/bus.py View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M src/ibustypes.h View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download

Messages

Total messages: 16
Daiki Ueno
It would be useful if certain components are started automatically when other components requests. My ...
12 years, 6 months ago (2011-09-08 09:59:23 UTC) #1
Peng
https://codereview.appspot.com/4960060/diff/1/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/1/bus/dbusimpl.c#newcode1166 bus/dbusimpl.c:1166: &success); Why use signal to invoke start_service of register? ...
12 years, 6 months ago (2011-09-08 12:24:27 UTC) #2
Daiki Ueno
https://codereview.appspot.com/4960060/diff/1/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/1/bus/dbusimpl.c#newcode1166 bus/dbusimpl.c:1166: &success); On 2011/09/08 12:24:27, Peng wrote: > Why use ...
12 years, 6 months ago (2011-09-09 07:27:43 UTC) #3
Daiki Ueno
On 2011/09/09 07:27:43, Daiki Ueno wrote: > https://codereview.appspot.com/4960060/diff/1/bus/dbusimpl.c > File bus/dbusimpl.c (right): > > https://codereview.appspot.com/4960060/diff/1/bus/dbusimpl.c#newcode1166 ...
12 years, 6 months ago (2011-09-12 03:11:12 UTC) #4
Peng
Sorry for the late reply. https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c#newcode1139 bus/dbusimpl.c:1139: guint32 flags; What´s the ...
12 years, 6 months ago (2011-09-12 13:02:55 UTC) #5
Daiki Ueno
https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c#newcode1139 bus/dbusimpl.c:1139: guint32 flags; On 2011/09/12 13:02:55, Peng wrote: > What´s ...
12 years, 6 months ago (2011-09-13 08:09:01 UTC) #6
Peng
https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c#newcode1139 bus/dbusimpl.c:1139: guint32 flags; On 2011/09/13 08:09:01, Daiki Ueno wrote: > ...
12 years, 6 months ago (2011-09-13 12:31:03 UTC) #7
Daiki Ueno
https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c#newcode1139 bus/dbusimpl.c:1139: guint32 flags; On 2011/09/13 12:31:08, Peng wrote: > On ...
12 years, 6 months ago (2011-09-14 06:47:04 UTC) #8
Peng
https://codereview.appspot.com/4960060/diff/16001/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/16001/bus/dbusimpl.c#newcode1139 bus/dbusimpl.c:1139: g_variant_new ("(u)", 0)); When timeout happens, the method should ...
12 years, 6 months ago (2011-09-14 13:24:11 UTC) #9
Peng
https://codereview.appspot.com/4960060/diff/16001/bus/marshalers.list File bus/marshalers.list (right): https://codereview.appspot.com/4960060/diff/16001/bus/marshalers.list#newcode11 bus/marshalers.list:11: BOOL:STRING,UINT Is it still needed?
12 years, 6 months ago (2011-09-14 13:25:04 UTC) #10
Peng
On 2011/09/14 06:47:04, Daiki Ueno wrote: > https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c > File bus/dbusimpl.c (right): > > https://codereview.appspot.com/4960060/diff/8/bus/dbusimpl.c#newcode1139 ...
12 years, 6 months ago (2011-09-14 14:21:12 UTC) #11
Peng
https://codereview.appspot.com/4960060/diff/16001/bus/dbusimpl.c File bus/dbusimpl.c (right): https://codereview.appspot.com/4960060/diff/16001/bus/dbusimpl.c#newcode1600 bus/dbusimpl.c:1600: g_slist_free_1 (p); g_slist_delete_link() could remove the node from line ...
12 years, 6 months ago (2011-09-14 14:21:18 UTC) #12
Daiki Ueno
Thanks for the review and helpful advice. The issues should be addressed in CL 7. ...
12 years, 6 months ago (2011-09-15 07:46:05 UTC) #13
Peng
LGTM. Thanks. On 2011/09/15 07:46:05, Daiki Ueno wrote: > Thanks for the review and helpful ...
12 years, 6 months ago (2011-09-15 13:51:04 UTC) #14
Peng
BTW, I am not sure if this API is supported in ibus python library. If ...
12 years, 6 months ago (2011-09-15 13:52:17 UTC) #15
Daiki Ueno
12 years, 6 months ago (2011-09-16 01:08:17 UTC) #16
On 2011/09/15 13:52:17, Peng wrote:
> BTW, I am not sure if this API is supported in ibus python library. If not,
how
> about add it in a new CL?

I'm about to push CL 8 with that, thanks.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b