[Bug 27270] TplLogManager: clean up API

[bugzilla-daemon at freedesktop.org]

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

--- Comment #10 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-06-04 03:16:58 PDT ---
(In reply to comment #9)
> I've fixed most of the uncontroversial comments:
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-logger;a=shortlog;h=refs/heads/manager-api-27270

I have a few complaints about the date manipulation, but overall this is a big
improvement.

> +      DEBUG ("Looking up date %u/%u/%u", g_date_get_day (date),
> +          g_date_get_month (date), g_date_get_year (date));

Ugh. d/m/y dates are ambiguous (Europeans interpret them as intended, Americans
interpret them as m/d/y and get confused). Please use ISO-8601 in all "C
locale" contexts, like debug messages:

      DEBUG ("Looking up date %04u-%02u-%02u", g_date_get_year (date),
          g_date_get_month (date), g_date_get_day (date));

> + * Returns: a GList of (GDate *), to be freed using something like
> + * g_list_foreach (lst, g_data_free, NULL);

Typo: g_date_free.

_tpl_log_manager_get_messages_for_date,
tpl_log_manager_get_messages_for_date_async,
tpl_log_store_get_messages_for_date and the get_messages_for_date virtual
method should all take a const GDate * parameter where they formerly used const
gchar.

> +#define _XOPEN_SOURCE /* glibc2 needs this for strptime */

This should be a hint that you're doing something non-portable, and indeed
Windows doesn't have strptime (this has bitten us before, in Gabble and
telepathy-glib). Thankfully, you don't need it: you can easily reimplement
create_date_from_string() by using sscanf() (which is ISO C) to parse a single
unsigned int, then using (u % 100), ((u / 100) % 100) and (u / 10000) as the
day, month, year, and failing if !g_date_valid_dmy().

> +              DEBUG ("Found text:'%s' in file:'%s' on date:'%u/%u/%u'", text,
> +                  hit->filename, g_date_get_day (hit->date),
> +                  g_date_get_month (hit->date), g_date_get_year (hit->date));

ISO date again, please.

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