(or longer for people further east than GMT+1...)
Of course the other solution is to go to bed *before* looking at
today's task, I totally agree :)
(or longer for people further east than GMT+1...)
Of course the other solution is to go to bed *before* looking at
today's task, I totally agree :)
Starting zanshin just after midnight, now today's tasks appear.
No Linters Available |
No Unit Test Coverage |
Oh right, would better use Utils::DateTime::currentDateTime() or similar. This one is overridable through the env var ZANSHIN_OVERRIDE_DATETIME.
The more I dig into this, the more I think all of my previous commits on the topic of timezones might be wrong.
This whole idea of using UTC everywhere comes from 9e65b991 / https://phabricator.kde.org/D876 which doesn't really tell me much about the reasoning.
As a user, I expect my workday to update at midnight LOCAL TIME, not midnight UTC.
All this code using UTC everywhere is, in fact, just wrong. The proper datetime for midnight (which is when the tasks start) is in local time, not in UTC.
We need to make a decision: if we're only supporting dates, we can simplify all this and use QDate everywhere.
If however we want to support date/times at some point, then we need to stop using UTC everywhere in this code, and use local time instead, since that's what we really mean.
So .... what was the reasoning behind the switch to UTC in https://phabricator.kde.org/D876?
Just a couple of minor issues
src/akonadi/akonadiserializer.cpp | ||
---|---|---|
295 | You kept the #if above, I'd say either it needs being kept here too or it needs being removed above. | |
src/presentation/artifactfilterproxymodel.cpp | ||
111 | Probably needs a rename to validDate now. | |
116 | And what if we reach year 10000? Did you think of the famous year 10000 bug??? :-) | |
tests/testlib/gentodo.cpp | ||
29 | Keep the #if here or remove it everywhere. | |
112 | ditto | |
tests/units/akonadi/akonadiserializertest.cpp | ||
48 | ditto |
src/akonadi/akonadiserializer.cpp | ||
---|---|---|
295 | That's because of the implicit conversion from QDateTime to KDateTime, so this code should work with both versions. But I haven't tested compilation with old kcalcore, so this is mostly guessing based on the old code. |
Confused that you didn't update this patch but introduced a new one, but fair enough the whole series looks good together.
You know I was joking for the 10K bug, right? :-)
I did both, updating this one to fix what you requested *and* creating a separate patch for the final cleanups (which include kcalcore-version-dependent code which has nothing to do with this bugfix, like very old QEXPECT_FAIL lines).