[Merge] lp:~smd-seandavis/ubuntu/trusty/xfce4-indicator-plugin/upstart-init into lp:ubuntu/xfce4-indicator-plugin

Dmitry Shachnev mitya57 at gmail.com
Wed Apr 9 14:11:38 UTC 2014


Review: Approve

Thanks for your merge proposal. This looks good, but I can't upload it right now, so I'll just leave some comments:

> gchar *INDICATORS_CMD[] = {"init", "--user", "--startup-event", "indicator-services-start", NULL};

That command exits immediately, doesn't it? What's the point of storing its PID?

> g_warning ("Failed to run \"indicator-services\": %s", error->message);

That warning message is a bit confusing, as the executable name is not "indicator-services".

Also, can you consider using libupstart in the future? It provides APIs to emit signals without forking (see how unity-panel-service starts the indicators).
-- 
https://code.launchpad.net/~smd-seandavis/ubuntu/trusty/xfce4-indicator-plugin/upstart-init/+merge/214713
Your team Ubuntu branches is requested to review the proposed merge of lp:~smd-seandavis/ubuntu/trusty/xfce4-indicator-plugin/upstart-init into lp:ubuntu/xfce4-indicator-plugin.



More information about the Ubuntu-reviews mailing list