[Bug 33208] New: write a set of Call base classes by extracting code from gabble and stabilize them for inclusion in tp-glib.

[bugzilla-daemon at freedesktop.org]

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

           Summary: write a set of Call base classes by extracting code
                    from gabble and stabilize them for inclusion in
                    tp-glib.
           Product: Telepathy
           Version: unspecified
          Platform: Other
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: medium
         Component: tp-glib
        AssignedTo: telepathy-bugs at lists.freedesktop.org
        ReportedBy: david.laban at collabora.co.uk
         QAContact: telepathy-bugs at lists.freedesktop.org


There's probably already a bug for this but I couldn't find it. Mark this as a
duplicate if you find it.

This bug is to track the progress of telepathy-yell and to discuss branches
(specifically
http://git.collabora.co.uk/?p=user/sjoerd/telepathy-yell.git;a=shortlog;h=refs/heads/media-fixes
and
http://git.collabora.co.uk/?p=user/sjoerd/telepathy-gabble.git;a=shortlog;h=refs/heads/call-base-media-content.



doing a git diff in yell between sjoerd/base-call-channel and
alsuren/base-call-channel-quick-wip shows the following problems:

#include "base-call-channel.h" should be the first #include, just as an extra
test of library usability. This is a trivial change, because your header is
properly formed anyway.


+static void
+base_call_channel_signal_call_members (TpyBaseCallChannel *self,
+  TpHandle removed_handle)
signals at most one removal and pretends that all of the members have changed.
Seems a bit ham-fisted.

Otherwise yell base-call-channel looks good. Be sure to fix the commit summary
"Include global tp-gglig header for TpDBusPropertiesMixi..." though.


gabble 614a4139032e1ed4efc8b47334b1c9d281354854
+  DEBUG ("Setting %d property on %p", property_id, self);
+
looks like temporary code put in place to fix a bug that is now
unnecessary+incomprehensible.

da5372b327ca015d9d670911b44f6d108324b9f1
- * Copyright (C) 2009 Collabora Ltd.
+ * Copyright (C) 2009-2010 Collabora Ltd.
It's 2011 now :P


da5372b327ca015d9d670911b44f6d108324b9f1
+  if (error != NULL || priv->deinit_has_run ||
+      priv->current_offer != TPY_CALL_CONTENT_CODEC_OFFER (source))
+    goto out;
is a bit confusing, especially given that you special-case each of these
conditions separately in out. I'm not completely convinced that you handle the
current_offer != source case correctly here. I think it will just cause a
confusing error in next_offer()

Would it be possible to give each error case its own guard statement to make
the code clearer?

-- 
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.