[Bug 26374] muc-calls branch review notes.

[bugzilla-daemon at freedesktop.org]

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

--- Comment #13 from Mike Ruprecht <cmaiku at gmail.com> 2010-05-21 22:50:33 PDT ---
(In reply to comment #12)
> > Don't let the async operation try to remember the request token
> > 6a1acd2cd7066ca9006177c3558680b1a5faa8cf
> > TpWeakRef could be used here. Would that be more beneficial in any way?
> 
> As a data structure you mean or ?

The object, but I looked at the object again and its more for handling if the
object is freed before the async request finishes. Since that's not happening
here, it's not really any extra useful.

> > Add basic support for removing members
> > ab0989e9bf2da328aeeb36e1a29ac78147e9d899
> > Adding a handle to base_call_channel_signal_call_members seems
> > hacky/non-obvious. Being that it's a static function, that's probably ok,
> > just feels weird.
> 
> Would you suggest an array instead? In practise members always leave one by
> one, 
> so a single TpHandle seems better. The other option is to only signal
> CallMembersChanged
> with a non-empty list of removals in gabble_base_call_channel_remove_member
> directly, but 
> that just seems like it would duplicate code for no good reason.

All I was saying was that it was not very obvious as to what the handle was
for, being that it's called "handle". I didn't realize quite what this was
doing though. Being that it's for emitting the CallMembersChanged, it makes
less non-obvious. It should be fine as is then.

Looks good and all tests pass make check :)

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