[Bug 21594] Implement being a Handler

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri May 15 20:51:43 CEST 2009


http://bugs.freedesktop.org/show_bug.cgi?id=21594





--- Comment #3 from Will Thompson <will.thompson at collabora.co.uk>  2009-05-15 11:51:43 PST ---
Very slight review so far:

AbstractClientApprover:
/* TODO add more methods */

What's with the comments in ChannelRequest::Private::Private?
+        QSet<uint>() << 0,                                           //
makesSenseForStatuses
+        Features(),                                                  //
dependsOnFeatures
+        QStringList(),                                               //
dependsOnInterfaces

I can't see how you're supposed to construct a ChannelRequest without having
its immutable properties. It doesn't start readying itself, and there's no
constructor that lets you omit the properties. Maybe this is just incomplete?

+ClientHandlerAdaptor::ClientHandlerAdaptor(const QDBusConnection &bus,
+        AbstractClientHandler *client,
+        QObject *parent)
+    : QDBusAbstractAdaptor(parent),
+      mBus(bus),
+      mClient(client)
+{
+    QList<ClientHandlerAdaptor *> &handlerAdaptors =
+        mAdaptorsForConnection[qMakePair(mBus.name(), mBus.baseService())];
+    handlerAdaptors.append(this);
+}

+ * One way to create a ClientRegistrar object is to just call the create
method.
+ * For example:
+ *
+ * \code ClientRegistrarPtr cr = ClientRegistrar::create(); \endcode
+ *
+ * You can also provide a D-Bus connection as a QDBusConnection:
+ *
+ * \code ClientRegistrarPtr cr =
ClientRegistrar::create(QDBusConnection::sessionBus()); \endcode

Which bus does create() use? It should be documented.

+    QString busName = QLatin1String("org.freedesktop.Telepathy.Client.");

it's not latin 1! it's ascii! And just below that a QString is appended. Is
that right?

+    // export o.f,T,Client interface

those commas should be dots.


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



More information about the telepathy-bugs mailing list