Code review - Issue 5716045: ibus-setup: improve ibus-daemon auto starthttps://codereview.appspot.com/2012-03-02T05:17:20+00:00rietveld
Message from unknown
2012-03-01T03:08:24+00:00Daiki Uenourn:md5:bb1a28c6f716c687fce35c20a05914c9
Message from daiki.ueno@gmail.com
2012-03-01T03:09:14+00:00Daiki Uenourn:md5:ed37b8a8de06e03d397466d07a1fac95
Message from takao.fujiwara1@gmail.com
2012-03-01T08:06:27+00:00fujiwaraurn:md5:6fe54010f254369e7749be98fa4af2cd
On 2012/03/01 03:09:14, Daiki Ueno wrote:
Not sure why you're working on this while I take the responsibility.
The main problem is fixed and the patch looks ugly for me.
Personally I hope you cancel this patch.
Message from daiki.ueno@gmail.com
2012-03-01T08:18:51+00:00Daiki Uenourn:md5:a7a01701563fdf83c940838b9c39232c
On 2012/03/01 08:06:27, fujiwara wrote:
> On 2012/03/01 03:09:14, Daiki Ueno wrote:
>
> Not sure why you're working on this while I take the responsibility.
> The main problem is fixed and the patch looks ugly for me.
> Personally I hope you cancel this patch.
Hmm, I think this patch is still worth being considered. It looks for me the current code is rather ugly, because:
* IBus.Bus() call never raise exceptions so try...except and checking the return value are not necessary at all
* I don't see any benefit of launching ibus-daemon multiple times. Asking user multiple times is generally not a good UI, IMHO
Message from Shawn.P.Huang@gmail.com
2012-03-01T14:29:13+00:00Pengurn:md5:958aac1cabeb7ec8700c6b5647324d31
http://codereview.appspot.com/5716045/diff/1/setup/main.py
File setup/main.py (right):
http://codereview.appspot.com/5716045/diff/1/setup/main.py#newcode372
setup/main.py:372: self.__bus.connect('connected', connected_cb, main_loop)
I suggest using main_loop.quit or (lambda *args: main_loop.quit()) as callback for those two function calls.
And then you may check bus.is_connected() after main_loop.run(), and then show different dialogs and etc.
I think it can make function a little clearer.
Message from unknown
2012-03-02T01:04:36+00:00Daiki Uenourn:md5:c5e49c5d88bab872f8659fe0f48ee897
Message from daiki.ueno@gmail.com
2012-03-02T01:07:11+00:00Daiki Uenourn:md5:0879708e55745a019efff2a349e0115f
On 2012/03/01 14:29:13, Peng wrote:
> http://codereview.appspot.com/5716045/diff/1/setup/main.py
> File setup/main.py (right):
>
> http://codereview.appspot.com/5716045/diff/1/setup/main.py#newcode372
> setup/main.py:372: self.__bus.connect('connected', connected_cb, main_loop)
> I suggest using main_loop.quit or (lambda *args: main_loop.quit()) as callback
> for those two function calls.
>
> And then you may check bus.is_connected() after main_loop.run(), and then show
> different dialogs and etc.
>
> I think it can make function a little clearer.
Yes, it certainly makes the logic easy to understand. Fixed.
Message from Shawn.P.Huang@gmail.com
2012-03-02T05:17:20+00:00Pengurn:md5:3dd7eb0a3861f11be222468d2c166890
lgtm with one comment
https://codereview.appspot.com/5716045/diff/4001/setup/main.py
File setup/main.py (right):
https://codereview.appspot.com/5716045/diff/4001/setup/main.py#newcode325
setup/main.py:325: # self.__config.connect("value-changed", self.__config_value_changed_cb)
Please remove those comment out code.