[Bug 46484] Add high-level Call bindings

[bugzilla-daemon at freedesktop.org]

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

--- Comment #13 from Andre Moreira Magalhaes <andrunko at gmail.com> 2012-03-20 11:41:02 PDT ---
Ok, here goes another review round.

There is just one issue apart from some nitpickings (below) that I believe we
should really fix before merging.
CallChannel is parsing immutable props on construction, but if the channel is
constructed directly without passing immutable props (in which case it should
still work), some accessors will be broken, such as initialTransportType(). So
please, add a proper FeatureCore (not just an alias for Channel::FeatureCore)
that will check if some immutable prop is missing and if so, retrieve them via
dbus. Check Channel::Private::introspectMainProperties for reference.

Some nitpicking:

diff --git a/CMakeLists.txt b/CMakeLists.txt
+set(FARSTREAM_MIN_VERSION "0.1.0")
+find_package(Farstream)
+macro_log_feature(FARSTREAM_FOUND "Farstream"
+                  "A Framework for dealing with audio/video conferencing
protocols"
+                  "http://www.freedesktop.org/wiki/Software/Farstream" FALSE
"0.1.0"
+                  "Needed, together with GStreamer and Telepathy-Farstream, to
build telepathy-qt-farstream")
Please use FARSTREAM_MIN_VERSION in place of "0.1.0" here so we don't forget to
update when we bump the required dependencies.

+# Find tp-farsight
+set(TELEPATHY_FARSTREAM_MIN_VERSION "0.2.2")
+find_package(TelepathyFarstream)
+macro_log_feature(TELEPATHYFARSTREAM_FOUND "Telepathy-Farstream"
+                  "A Framework for dealing with audio/video conferencing
protocols"
+                  "http://telepathy.freedesktop.org/wiki/" FALSE "0.2.2"
Same here

diff --git a/TelepathyQt/Farstream/CMakeLists.txt
b/TelepathyQt/Farstream/CMakeLists.txt
+    # We are building Telepathy-Qt4-Yell-Farstream
*Telepathy-Qt

diff --git a/TelepathyQt/Farstream/TelepathyQtFarstream.pc.in
b/TelepathyQt/Farstream/TelepathyQtFarstream.pc.in
{QT_MIN_VERSION}, QtDBus < ${QT_MAX_VERSION}, telepathy-glib >= 0.17.5,
telepathy-farstream >= 0.2.0, TelepathyQt${QT_VERSION_MAJOR} =
${PACKAGE_VERSION}
Please use macros for tp-glib/farstream versions. I noted also that we have
hardcoded version in TpFarsight.pc.in, it would be good if you could also
update it.
The same happens in the uninstalled versions (btw, do we really need them when
using cmake?). Tbh I guess we could just remove the uninstalled versions as
this in theory should be used by libtool only.

diff --git a/TelepathyQt/Farstream/channel.cpp
b/TelepathyQt/Farstream/channel.cpp
+namespace Tp {
+namespace Farstream {
\n{. Please also replace all other similar occurrences.

+    if (!channel->handlerStreamingRequired()) {
+        setFinishedWithError(TP_QT_ERROR_NOT_AVAILABLE,
+                QLatin1String("Handler streaming not required"));
+        return;
+    }
Please add some debug warnings here also and in other early-returns.

+CallChannelPtr PendingChannel::callChannel() const
+{
+    return CallChannelPtr::staticCast(object());
Please use qObjectCast here.

diff --git a/TelepathyQt/channel-factory.h b/TelepathyQt/channel-factory.h
There are some extra blank lines there that could be removed.

Apart from those, excellent work, feel free to merge once the above issues are
addressed.

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