[Bug 27270] TplLogManager: clean up API

[bugzilla-daemon at freedesktop.org]

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

--- Comment #11 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-06-04 05:29:07 PDT ---
(In reply to comment #10)
> (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));

done.

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

fixed.

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

done.

> > +#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().

done.

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

fixed.

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