Bug 34983

My implementation:

http://git.collabora.co.uk/?p=user/nicolas/telepathy-logger.git;a=shortlog;h=refs/heads/date-time

This looks pretty good.

I had originally wondered if you should just pass GDateTimes around in the API, but then you have to manage their memory which seems like a pain in the neck.

The only potential downfall I can see of using gint64s throughout is that you could potentially mix up localtime and unix time? You code here looks correct, but it's a mistake someone could possibly make in the future? This could potentially be mitigated with a TplUnixTimestamp64 typedef, although it then requires people to use it and think and understand, so maybe it's not useful.

Does this branch remove all of the time_ts from the API? I thought you'd said there were still some left in the public API, but greping didn't find them.

(In reply to comment #2)
> This looks pretty good.
> 
> I had originally wondered if you should just pass GDateTimes around in the API,
> but then you have to manage their memory which seems like a pain in the neck.

Actually, one of my goal is to replace the gint64 timestamp in TplEvent with a GDateTime. As GDateTime is immutable and ref-counted, I never have to copy it. As most operation simply require generating formated strings I don't even have to reference it. But this requires breaking API, so I decided to do it in two steps, this is first and second is to actually break the API and integrate it in future 0.3.X branch.

> 
> The only potential downfall I can see of using gint64s throughout is that you
> could potentially mix up localtime and unix time? You code here looks correct,
> but it's a mistake someone could possibly make in the future? This could
> potentially be mitigated with a TplUnixTimestamp64 typedef, although it then
> requires people to use it and think and understand, so maybe it's not useful.

It's indeed a risk, GDateTime is even a better solution since it abstract the timezone. Creating TplUnixTimestamp64 does not seem bad, and does not actually break the API, I'll consider this.

> 
> Does this branch remove all of the time_ts from the API? I thought you'd said
> there were still some left in the public API, but greping didn't find them.

There was no time_t in the public API, but this change removes all use of time_t in the internal implementation. For 0.3.X I also plan on replacing GDate with GDateTime in list return by _get_dates() and inside TplSearchHit.

(In reply to comment #3)

> Actually, one of my goal is to replace the gint64 timestamp in TplEvent with a
> GDateTime. As GDateTime is immutable and ref-counted, I never have to copy it.
> As most operation simply require generating formated strings I don't even have
> to reference it. But this requires breaking API, so I decided to do it in two
> steps, this is first and second is to actually break the API and integrate it
> in future 0.3.X branch.

Ahh, so it will return a (transfer none) pointer to the GDateTime that's owned by the object?

> It's indeed a risk, GDateTime is even a better solution since it abstract the
> timezone.

That was my thinking.

> There was no time_t in the public API, but this change removes all use of
> time_t in the internal implementation. For 0.3.X I also plan on replacing GDate
> with GDateTime in list return by _get_dates() and inside TplSearchHit.

Is there a problem with using GDate, it hasn't been deprecated afaik, and doesn't have wrapping problems, since it's just a struct. It makes sense for get_dates() because it's just a date (no time). It is mutable, but we only ever pass copies as transfer-full right?, so that's not a big deal. If you did use GDateTimes what timezone would you use in them?

(In reply to comment #4)
> (In reply to comment #3)
> 
> > Actually, one of my goal is to replace the gint64 timestamp in TplEvent with a
> > GDateTime. As GDateTime is immutable and ref-counted, I never have to copy it.
> > As most operation simply require generating formated strings I don't even have
> > to reference it. But this requires breaking API, so I decided to do it in two
> > steps, this is first and second is to actually break the API and integrate it
> > in future 0.3.X branch.
> 
> Ahh, so it will return a (transfer none) pointer to the GDateTime that's owned
> by the object?
Yes.

> > There was no time_t in the public API, but this change removes all use of
> > time_t in the internal implementation. For 0.3.X I also plan on replacing GDate
> > with GDateTime in list return by _get_dates() and inside TplSearchHit.
> 
> Is there a problem with using GDate, it hasn't been deprecated afaik, and
> doesn't have wrapping problems, since it's just a struct. It makes sense for
> get_dates() because it's just a date (no time). It is mutable, but we only ever
> pass copies as transfer-full right?, so that's not a big deal. If you did use
> GDateTimes what timezone would you use in them?

Actually your right, only the time part of GDate has been deprecated. GDate can
be kept. Also, it allow not to care about timezone and risk that the represented 
date changes because of current locale settings. The goal of those dates are to
present days as the user remember, so regardless of the timezone.

Merged, will be 0.2.6.