Code review comment for lp:~bratsche/indicator-session/users-service

I'm going to sleep, but figured I'd document the status of this now.

> * In users-service.c I don't see where count is updated. I think
> it should be incremented/decremented on user added/removed.

Fixed.

> * There should be icons on the users if they are set (probably
> should go in another branch/bug)

TODO. Can we do this in a separate branch?

> * There doesn't seem to be any sorting of the user list. They
> should be sorted alphabetical by first name.

Fixed.

> * There are six signals defined in the Class structure but only 4
> created on class init.

TODO.

> * I don't think that you need a call to
> dbus_g_object_type_install_info, that should only be needed if
> this object is being shared over DBus (server) but I think it's
> only a client.

Fixed.

> * Confirm with Robert, but I'm pretty sure the excludes list is
> already being applied on the GDM side. We shouldn't need to
> apply it on this side as well. That way there's only one copy
> of the list.

Confirmed with Robert, and fixed.

> * I think that dbus_g_connection_register_g_object can be dropped
> as well. Which I think is the only reason you're using the
> session bus, so I think that can be dropped.

Fixed.

> * Oh, now realizing more of the issue. Makefile.am shouldn't even
> be generating user-service-server.h. Drop that file.

Fixed.

> * I don't think that dbus_proxy_system and dbus_proxy_session are
> needed as we're not asking DBus itself for any information.
> We're just using it to communicate. We'd only need those
> proxies to query information about the bus itself.

Fixed.

> * Can session_compare be replaced with g_strcmp0? It seems about
> the same.

Fixed.

> * I was looking at "CanActivateSessions" call to ConsoleKit. Do
> you think we should check this earlier and make the menu items
> sensitive or insensitive based on this?

See previous comment.

> * It looks like you're always getting the information on all the
> users. I think that we really don't care if it's over our max,
> so can we just not bother getting all that data if we're in
> overflow?

Fixed.

> * The guest user needs to be handled as well. If there is a guest
> already logged in selecting the guest on the menu shouldn't
> start a new session, but should go to the guest that is already
> running.

TODO.