[Bug 23963] Support for presence optimisations

[bugzilla-daemon at freedesktop.org]

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

--- Comment #17 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2011-03-16 07:55:31 PDT ---
Looks pretty good! I've just got a few style comments it'll take no time to fix
up.

Senko's updated wocky branch is:

http://git.collabora.co.uk/?p=user/ptlo/wocky/.git;a=shortlog;h=refs/heads/stanza-queueing-c2s

(In reply to comment #15)
> wocky:

+  gboolean power_saving_mode;
+  /* Queue of (owned WockyStanza *) */
+  GQueue *unimportant_queue;
+  /* List of (owned WockyStanza *) */
+  GList *queueable_stanza_patterns;
+
+
   WockyXmppConnection *connection;

I like your comments, nice going. Now just remove the extra newline. :-)

+  for (l = priv->queueable_stanza_patterns; l; l = l->next)
+      g_object_unref (WOCKY_STANZA (l->data));

Bad indentation, should only be two spaces, and "l" should be "l != NULL". Also
g_object_unref takes a gpointer, you don't need to cast the data to a
WockyStanza. But... you could just replace this with:

g_list_foreach (priv->queueable_stanza_patterns, (GFunc) g_object_unref, NULL);

and you'd avoid the annoying GList *l, and the typecheck.

+  while (NULL != (stanza = g_queue_pop_head (priv->unimportant_queue)))

This takes a little while to read. For clarity in a few years, could you write
it out as something like:

for (stanza = q_queue_pop_head (...); stanza != NULL; stanza = g_queue_pop_head
(...))

Hm, that's unfortunate we have to call pop_head twice. Well, yeah, or something
like that.

+ * Enable or disable power saving. In power saving mode, Wocky will
+ * attempt to queue "uninteresting" stanza until it is either manually
+ * flushed, until important stanza arrives, or until the power saving
+ * mode is disabled.

It'd be nice if you could add a few notes as to what uninteresting stanzas are
please.

+void wocky_c2s_porter_flush_unimportant_queue (WockyC2SPorter *porter);

Does this need to be public?

+is_stanza_important (WockyC2SPorter *self, WockyStanza *stanza)

Each argument on a new line for declarations.

+      if (!ptype || !wocky_strdiff (ptype, "unavailable"))

ptype != NULL

+  if (!priv->queueable_stanza_patterns)

priv->queueable_stanza_patterns != NULL

+  for (l = priv->queueable_stanza_patterns; l; l = l->next)

l != NULL

Other than those little niggles it's basically fine.

(In reply to comment #15)
> gabble:

+#include <wocky-namespaces.h>
+#include <wocky-c2s-porter.h>

#include <wocky/wocky-foo.h> please. I'm actually kind of surprised that
worked?

All your other gabble changes look good.

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