Code review comment for lp:~ted/indicator-session/upstart-job

Revision history for this message
Sebastien Bacher (seb128) wrote :

> I figured this was a good way to start, as the change may break things :-) I think that we should look at removing it as we start to get more stable.

you expect the upstart change to trigger code bugs? that's debugging mode and we don't want to have to patch all jobs at beta time ... having upstart exporting that variable for all jobs during unstable cycles seems like it would be a better way?

> The biggest driver for the change today is from QA. They want to be able to shutdown the individual services without having them restart so that they can replace them with mocks.

that's fair enough, still I would prefer if we could resolve the current env issues we have before adding extra jobs (we still have e.g indicator-session running gnome-control-center with the wrong XDG_CURRENT_DESKTOP sometimes, which leads to have unity controls not listed)

> Uhm, that's interesting. I'm not sure what the mask should be there, != ubuntu-touch ?

that's a good question, I'm not sure either what's the right set ... "!= ubuntu-touch" seems wrong though, afaik indicators are only used in Unity, gnome-panel, xfce (and maybe lxde), and I'm not sure if indicator-session is being using in xfce/lxde (they use e.g -sound I think). We should probably reach to flavor being doing those changes, as we might create issues for them otherwise

review: Needs Fixing

« Back to merge proposal