[local-time-devel] [PATCH] Make local-time operate correctly with timezones

Antoni Piotr Oleksicki apoleksicki at o2.pl
Sat Aug 8 14:23:06 UTC 2009


Hi Attila,

2009/7/29 Attila Lendvai <attila.lendvai at gmail.com>:
> i've pushed a patch to the official repo that is non-controversial,
> and pushed the rest to my local tree to start testing it locally.
> my timezone related knowledge is not enough to review the patch, but
> your mail is starred and i'll start using your changes.

Great! We'll be awaiting your feedback.

> after a really short glimpse i didn't like the introduction of the
> constant +none-zone+. i'd much prefer seeing assert's all around where
> both arguments are available, so that calling a public api function
> with both a timezone and an offset results in an error signaled.
> something along the line of this as the first line of each such
> function:

> (defmacro assert-proper-timezone-and-offset ()
>  `(assert (or (not timezone-provided?) (not offset))))

The assert idea is good, but it is not why +none-zone+ is there. It is
because previously some of the code was called without any timezone or
offset arguments and simply assumed zero offset, which happens to be
+UTC-zone+, which was then used. This was really rather messy and
introducing +none-zone+ was the easiest way to signify an explicit
offset without disturbing the rest of the code too much. We're not too
happy about +none-zone+ either, but doing away with it would require a
lot more changes, quite possibly API-breaking ones.

> and for the internal functions using NIL's for the uninteresting position.

Yep. There are more cleanup possibilities, amongst them reducing a lot
of unnecessary duplication like what TIMESTAMP+ and TIMESTAMP- used to
have. But we didn't want to go too far this route without first
checking that it's going to have at least some chance of being
accepted :).

Cheers,
Antoni & Maciej




More information about the local-time-devel mailing list