[Bug 30338] TpRoomList - High level API for RoomList channel

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Thu Apr 26 10:31:59 CEST 2012


https://bugs.freedesktop.org/show_bug.cgi?id=30338

--- Comment #16 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2012-04-26 01:31:59 PDT ---
(In reply to comment #15)
> TpRoomInfo:
> 
>  - SECTION doc refers to TpRoomListChannel which does not exist

fixed.

>  - I know this is common, but GObjectClass guarantees that finalize/dispose/etc
> are non-NULL, they implement them explicitly to not having to check in every
> subclass.
> +  void (*chain_up) (GObject *) =
> +      ((GObjectClass *) tp_room_info_parent_class)->finalize;

IIRC was advocating for some reason but I don't remember the details.

>  - tp_room_info_get_members() I was surprised it return a number, I was
> expecting getting a list of TpContact from the func name... maybe
> _get_n_members()? or _get_members_count()?

Renamed to get_members_count().

>  - Shouldn't it expose each getter as a property as well?

Not sure it's worth it as they are not 'construct'. The only advantages I see
is to get all the 'props' in one g_object_get() call but I hate this as it's
not safe at all.

>  - I'm not fan of self->priv->hash name, it does not tell what it contains.
> 'asv' would already describe it better... or "parameters" maybe?

Agreed. I renamed it 'info' as that's the name used in the spec. 

> TpRoomList:
> 
>  - SECTION doc says it's a TpChannel subclass... it's not.

fixed.

>  - "got-rooms" signal emits only one room at a time, right? so no plurial
> needed.

Good point; renamed.

>  - tp_room_list_get_listing() --> _is_listing() ?

I meant to change that but forgot; fixed.

>  - I think it should "keep its state". I mean that if you miss signals, you
> can't know the rooms listed, or the fail error... I would like a getter for
> those 2.

How can you miss it? They won't be fired until _star() is called so you are
supposed to connect the signals before.

> simple-conn.c:
> 
>  - chan = g_hash_table_lookup (self->priv->channels, GUINT_TO_POINTER (0));
> WTF using 0 as key?? is that really on purpose?

Yeah, room list channel has None as HandleType so no Handle either.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list