[Bug 46484] Add high-level Call bindings

[bugzilla-daemon at freedesktop.org]

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

Dario Freddi <drf54321 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |r-

--- Comment #2 from Dario Freddi <drf54321 at gmail.com> 2012-03-13 05:22:20 PDT ---
Review from my side, besides a ++ to all of Andre's comments. Not really
thorough as I did it in a hurry, but still:

+    if (withVideo) {
+        request.insert(TP_QT_IFACE_CHANNEL_TYPE_CALL +
QLatin1String(".InitialVideo"),
+                       true);
+        if (!audioName.isEmpty()) {
+            request.insert(TP_QT_IFACE_CHANNEL_TYPE_CALL +
QLatin1String(".InitialVideoName"),
+                           audioName);
+        }
+    }

copy/pasta error on videoName vs. audioName

+                self->properties->GetAll(
+                    QLatin1String(TP_QT_IFACE_CHANNEL_TYPE_CALL)),

TP_QT_IFACE_CHANNEL_TYPE_CALL should be a QLatin1String already, no need to
recreate one. Please check for other occurrences of that, there are a few.

- I don't get why you have several features which are basically getting all of
the properties for the same interface. As long as Content makes some sense
since it just pulls a single property, features CallState and CallMembers
basically just do the same thing, just storing a different set of properties in
the end. Given that the whole purpose of Features is to save DBus roundtrips,
wouldn't it make more sense to merge at least State and Members together? (I'd
be in favor of merging Contents as well, but there might be a use case for it)

+    return interfaces().contains(TP_QT_IFACE_CALL_CONTENT_INTERFACE_DTMF);

Nitpick: maybe use hasInterface()?

+            Q_ASSERT(mPriv->remoteMembersContacts.contains(handle));

I guess we don't want to assert here


Overall looks really good though :)

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