hpi-swa / Squot

Squeak Object Tracker - Version control for arbitrary objects, currently with Git storage
Other
57 stars 29 forks source link

Timezone of commit timestamps in GitBrowser wrong #294

Closed LinqLover closed 2 years ago

LinqLover commented 3 years ago

This

image

does not match this

https://github.com/hpi-swa-teaching/AutoTDD/pull/14/commits/f7ccede9711804886d667f05fe880657bf3d1a03 image

I would prefer it if the timestamps in the GitBrowser could be displayed in the local timezone instead of UTC.

LinqLover commented 2 years ago

... on the other hand, this gets more complicated for summertime/wintertime offsets ... GitHub converts all timezones to my current German summertime (MESZ), but not times in MEZ (German wintertime) ... Hard to find a good heuristic for this. 😅

j4yk commented 2 years ago

Funnily, Git's own programs also disagree with each other:

image

The commit timestamp includes a time zone offset, which is 0 for this particular commit, whereas it is not in others:

Jakob@JAKOBS-PC MINGW64 /c/Squeak/AutoTDD (dev)
$ git cat-file commit f7ccede
tree 4a84de47c9ed452f903aed38ed7648be74a69d26
parent 41f6857a33dcc2a50f62dbe8b6eb99cd9f07ca4d
parent 4b65aca7fd175b18306b2099ac36413056f50192
author Christoph Thiede <christoph.thiede@outlook.de> 1602025430 **+0000**
committer Christoph Thiede <christoph.thiede@outlook.de> 1602025430 **+0000**

Merge master into dev

Updates baseline:Completely remove requirement to Regex package, which is in Trunk anyway

Jakob@JAKOBS-PC MINGW64 /c/Squeak/AutoTDD (dev)
$ git cat-file commit 41f6857
tree ebfc4562738dce9ca7d853fb4eaed4a6843b44a6
parent 605d56b4cf45eebae5fe70fcd09ded9564ef5578
author Patrick R <codeZeilen@users.noreply.github.com> 1583504706 **+0100**
committer GitHub <noreply@github.com> 1583504706 **+0100**
gpgsig -----BEGIN PGP SIGNATURE-----
...
j4yk commented 2 years ago

I don't know any good way to solve this except for accepting it as it is.

As you already noticed, we cannot simply convert all timestamps to the current time zone offset because then in winter all commits from the summer would look different than when you view them in summer and vice versa.

Squeak's Chronology classes seem to lack a facility to look up the offset of a past instant in a given time zone. The java.time class library introduced with Java 8 is as extensive as it is (i. e. much more extensive than Squeak Chronology) for a reason...

j4yk commented 2 years ago

Well, this was an interesting read: I wanted to look up how this would be solved outside of Squeak, using C.

Here is my (slightly reordered) reading trail:

The Wikipedia article even pointed me to a Squeak package maintained by Dave Lewis, so Squeak is not totally out of the loop: http://wiki.squeak.org/squeak/1076

But is another dependency really worth it? I don't think so, but maybe there is a way to use the package if it is already loaded.

LinqLover commented 2 years ago

Uh, this smells for exploding complexity ... Anyway, an interesting reading trail indeed ... Could we maybe just introduce a simple heuristic and display a simple warning message (something like "âš  different timezone") next to the timestamp if its zone diverges from the current one? Or would even this detection be non-trivial? If yes, please just close this issue and let us try to forget these deep abysses ...

j4yk commented 2 years ago

Every piece of text that we add to that column will move all commit messages further to the right. So no warning message in the timestamp column... and I do not think that it would be self-explanatory to format a single "cell" in the list differently, even if it were possible.

We could add text to the summary which is displayed in the bottom-left corner of the Git Browser and in the tooltips of the commits in the list. But note that in locales with daylight saving time, you would get this warning displayed for commits from half of each year... so it could end up being more noisy than it is useful.

LinqLover commented 2 years ago

Okay, what would you think about #382? No explicit mental-overloading warnings but just an omission of the timezone if it is equal to the local timezone. If you think that this still creates confusion, though, we may just close this issue as "not planned". :-)