drop `ZonedTime` in favour of `UTCTime`
MangoIV opened this issue · 7 comments
Summary
This issue exists for evaluating the advantages and disadvantages of dropping ZonedTime in favor of UTCTime. The main reason I want this is that ZonedTime doesn't admit an Ord instances making it hard to use e.g. with containers. Additionally, not all timestamps output by git or parsed from toml are zoned, hence we're inventing a bunch of data which doesn't have to be the case. For me the question is what do we actually need the time zones for?
Blocked by glguy/toml-parser#19
I've got a PR open you can look at.
Note that TOML supports both of these two literals and they don't mean the same thing:
example1 = 1979-05-27T07:32:00Z
example2 = 1979-05-27T07:32:00You can already decode example2 as a Data.Time.LocalTime. With the PR you can decode example1 as a Data.Time.UTCTime
Those two examples aren't intended by TOML to be interchangeable. I can't think of any good reasons for security advisories to be using local times.
OSV uses UTCTime. But because we read timestamps from Git, our own advisory type uses ZonedTime, so that we don't throw away information. According to https://git-scm.com/docs/git-commit#_date_formats, all Git commits should have zone offset information. I take your point re TOML and interpretation of non-zoned data; but we could clarify and document the semantics, e.g. how a bare day or timestamp will be interpreted.
My 2c: I'd rather preserve this information in our internal types, and users can convert to UTCTime as and when needed (e.g. sorting).
If there is a clear consensus to switch, I won't oppose it. Can discuss and decide next week at ZuriHac.
But because we read timestamps from Git, our own advisory type uses ZonedTime, so that we don't throw away information
turns out, this assumptions isn’t correct. (Caused me some headaches)
My 2c: I'd rather preserve this information in our internal types, and users can convert to UTCTime as and when needed (e.g. sorting).
this complicates the interface significantly, none of the “internal” types (which are exposed via the interfaces so I don’t know whether they’re really internal) can have an Ord instance. Additionally the extra information is almost worthless as e.g. in the case of offset zero we don’t know if this actually comes from an utc time zone or was parsed without offset… (which happens in two different situations, I think)
According to https://git-scm.com/docs/git-commit#_date_formats, all Git commits should have zone offset information.
Where does it say that? I couldn’t find it.
Based on discussions and input from multiple parties, we will switch to UTCTime.