Code review comment for lp:~muktupavels/libappindicator/create-as-service

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

> On Wed, 2015-07-01 at 16:32 +0000, Alberts Muktupāvels wrote:
>
> > I don't like this spec, but I guess there is no better choice... Does
> > that mean that they have changed spec when there was existing
> > implementations?
>
>
>
> Yes, and I imagine that is why it is written as a "can" instead of a
> "must" or even "should."
>
> "Each application can register"
>
> http://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifi
> erItem/

I understand it like this:
Each application can register multiple items...

Next paragraph:
"As soon as a new instance of a StatusNotifierItem is created, the application must register the unique instance name to the StatusNotifierWatcher as described in the Section called StatusNotifierWatcher"

Is not unique name meant to be org.freedesktop.StatusNotifierItem-PID-ID?

RegisterStatusINotifierItem method:
"Register a StatusNotifierItem into the StatusNotifierWatcher, in the form of its full name on the session bus, for instance org.freedesktop.StatusNotifierItem-4077-1."

> > > You can watch for when the connection drops off the bus as well. I don't
> > > think there's a real difference between watching the name and the
> > > connection.
> >
> >
> > That was just small example. Still I think it is better to follow spec
> > not trying to support each possible implementation that is different
> > from spec.
>
>
>
> Since it is not a required part of the spec I think you should probably
> support the more generic DBus mechanisms as well as parts of the spec
> that you wish.

Well spec says exactly how item should be registered with watcher... I don't know how that spec looked in past - that is what I am reading right now.

> > > > Did not understand part about not allowing name registrations... :(
> > >
> > >
> > > For Ubuntu Personal and Ubuntu Phone we want to allow application
> > > indicators in the convergence cases, but the confinement there doesn't
> > > allow for name registrations on the dbus session bus.
> >
> >
> > Is there a way to detect that application is running on Ubuntu
> > Personal and/or Ubuntu Phone? Then I could update branch to register
> > dbus name only if it is not running on Personal or Phone.
>
>
> I think what would perhaps be better is allow for the name registration
> to fail, and handle that exception. There's no real reason to put the
> name in the messages. Basically make it so that the name request is only
> optional to have succeed.

https://developer.gnome.org/gio/stable/gio-Owning-Bus-Names.html#g-bus-own-name
That would be name_lost_handler with connection == NULL, right?

So, are you ok with this change? with adding fallback to old behaviour if name registration fails?

« Back to merge proposal