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

On Tue, 2013-08-27 at 19:01 +0000, Sebastien Bacher 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?

No, but I expect there were a lot of messages and bugs that we can debug
today that we couldn't before because all the log files were
intermingled and the messages and warnings got lost. I'm expecting
apport bugs to be much more useful and I'm trying to optimize that as we
start to fix and refine.

I'm not against making it a global setting. My only comment would be
there that probably most applications aren't looking at the logs, but we
would be for indicators. I have no problem generating them though. I
don't feel like that's a decision I can make, my field of influence is
the indicators :-)

> > 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)

We should fix those bugs, I agree. These branches have been around for
a while, and this change has been planned. It is later than I'd like,
but I'd rather get it done and move on.

> > 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

So let's leave it is not filtering right now. The reality is that the
phone won't have indicator-session installed in most cases, and even if
it was, it would only take a bit of RAM not really hurt anything.