Bug 77191

Summary:	[next] simplify presence mixin: GHashTable -> "has message?"
Product:	Telepathy	Reporter:	Simon McVittie <smcv>
Component:	tp-glib	Assignee:	Simon McVittie <smcv>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	guillaume.desmottes, xclaesse
Version:	git master	Keywords:	patch
Hardware:	Other
OS:	All
Whiteboard:	review+
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	68661, 76369

Description Simon McVittie 2014-04-08 17:43:17 UTC

+++ This bug was initially created as a clone of Bug #76369 +++

We should make sure that dbus-glib isn't exposed in our high-level API.

TpPresenceStatus has a dbus-glib data type, but should just have a boolean - "allows message?" - since everything else that could go there is no longer in a non-deprecated interface in 0.x, let alone 1.0.

Comment 1 Simon McVittie 2014-04-09 19:07:11 UTC

http://cgit.freedesktop.org/~smcv/telepathy-glib/log?h=presence-mixin
http://cgit.freedesktop.org/~smcv/telepathy-gabble/log?h=presence-mixin
http://cgit.freedesktop.org/~smcv/telepathy-salut/log?h=presence-mixin
http://cgit.freedesktop.org/~smcv/telepathy-haze/log?h=presence-mixin

Comment 2 Xavier Claessens 2014-04-09 20:26:23 UTC

+1 for the tp-glib commit 11c91762865545ab7d0704534ac7736d78795fdf. Didn't read rest of that branch, neither did I read CM branches.

We forgot to _TP_SEAL() those presence mixin structs. TpPresenceStatusSpec has all the getters needed to move the struct to presence-mixin.c but we should add getters for TpPresenceStatus fields. Doing that saves us all that padding. I did not grep CMs to know if that's possible to do at this point, though. That's not preventing to merge your patch first though.

Comment 3 Simon McVittie 2014-04-10 12:23:34 UTC

(In reply to comment #2)
> We forgot to _TP_SEAL() those presence mixin structs. TpPresenceStatusSpec
> has all the getters needed to move the struct to presence-mixin.c but we
> should add getters for TpPresenceStatus fields. Doing that saves us all that
> padding. I did not grep CMs to know if that's possible to do at this point,
> though.

It isn't possible without CM changes: some CMs put a TpPresenceStatus on the stack.

I think this is relatively unimportant: we can do a TpPresenceMixin2 at some point if we need to.

Comment 4 Simon McVittie 2014-04-10 13:00:34 UTC

Fixed in git for 0.99.11 (CM patches to follow when I've re-tested). I'll leave this bug open until I find the other bug I'm sure I filed already for your other suggestions.

Comment 5 Simon McVittie 2014-04-22 18:19:17 UTC

(In reply to comment #4)
> I'll
> leave this bug open until I find the other bug I'm sure I filed already for
> your other suggestions.

Bug #71508, to which I have attached my work in progress.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.