[Bug 32902] review Pidgin log store and dbus testsuite

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Jan 10 18:56:43 CET 2011


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

Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review- (need to cleanup
                   |                            |patchset)
           Keywords|patch                       |

--- Comment #1 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-01-10 09:56:42 PST ---
> +++ b/telepathy-logger/log-store-pidgin-internal.h
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright (C) 2008-2009 Collabora Ltd.
> + *

We are in 2011, same in .c file.

>diff --git a/telepathy-logger/log-store-pidgin.c b/telepathy-logger/log-store->pidgin.c
>+static gchar *
>+log_store_pidgin_get_dir (EmpathyLogStore *self,
>+  /* The escaping behaviour here was taken from Pidgin, obviously. */

This escaping is defined in RFC3986 (http://www.ietf.org/rfc/rfc3986.txt) and
is implemented in GLib, see g_uri_escape_string/g_uri_unescape_string. There is
also helpers for filesname to/from URI.

>+static GList *
>+log_store_pidgin_get_dates (EmpathyLogStore *self,
>+  if (directory == NULL)
>+    {
>+      g_free (directory);
>+      return NULL;
>+    }
No need to free directory, it's NULL (remove scope after)

Ok, I'll stop there for now. I see bunch of stuff that should be
rebased/squashed before asking for review. Please cleanup your patchset so
every changes mean somtehing. We don't need to know about all your intermediate
steps, especially for new files.

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



More information about the telepathy-bugs mailing list