> On Oct. 16, 2013, 12:15 p.m., John Layt wrote: > > Wow, there sure is a lot of code in there catering for all the possible > > corner cases :-) QTimeZone has a lot less places it checks, so I'll need > > to do an in-depth comparison, but given Qt5 will only support modern > > distros I think most of it is redundant. > > > > One thought is if we still want to have a signal that the system database > > has been updated? While Qt doesn't cache the time zone data, the apps > > probably will cache the QTimeZone instances themselves and may need to be > > told that the database has changed so they can take action. Then again, > > it's not something that will happen very often, and what will the apps do > > in response? If they refresh their cache the shared copies in QDateTime > > won't update. I guess QTimeZone will need a "reloadData()" method added > > instead. > > > > I've looked at the Windows code and it mostly looks OK, all it does is > > monitor the registry for changes. What you do need to change is the DBus > > signal from "configChanged" to your new "timeZoneChanged" signal. > > > > With regards the signal name change, does it need to go in the porting > > guide or somewhere so people know it has changed? > > > > In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and > > TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still > > need the daemon for now. > > Martin Klapetek wrote: > > Wow, there sure is a lot of code in there catering for all the possible > corner cases :-) QTimeZone has a lot less places it checks, so I'll need to > do an in-depth comparison, but given Qt5 will only support modern distros I > think most of it is redundant. > > Part of that is support for Solaris, but I see Solaris is not supported > by Qt anymore [1][2], so I just removed it altogether. > > > One thought is if we still want to have a signal that the system > database has been updated? While Qt doesn't cache the time zone data, the > apps probably will cache the QTimeZone instances themselves and may need to > be told that the database has changed so they can take action. > > I think it might be worth having it...I'll add it back, can't hurt :) > > > I've looked at the Windows code and it mostly looks OK, all it does is > monitor the registry for changes. What you do need to change is the DBus > signal from "configChanged" to your new "timeZoneChanged" signal. > > Ah yes, I forgot to add that to the patch, sorry. > > > With regards the signal name change, does it need to go in the porting > guide or somewhere so people know it has changed? > > I wanted to add it, but that's in kdelibs. Question is, should there be a > guide for runtime/workspace too? > > > In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and > TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still > need the daemon for now. > > Ok, keep us posted. (It would be cool to have cross-desktop solution for > this too...or system-wide spec at least, so we could use non-qt/-kde apps > still supporting timezone changes, but oh well). > > > [1] - http://qt-project.org/doc/qt-5.1/qtdoc/supported-platforms.html > [2] - http://qt.digia.com/Product/Supported-Platforms/ > > David Faure wrote: > There are still people around with a Solaris system. Not supported > doesn't mean we should actively remove existing support, IMHO. > > About the porting guide: the distinction between kdelibs and kde-runtime > will disappear, given the framework-ification. So treat it as a porting guide > "from kde 4 to kde-frameworks", i.e. it's fine to include "runtime" stuff in > there. > > About a solution in Qt --- that means each and every Qt app will have to > monitor the same timezone file(s), isn't that a bit too expensive? (not sure, > just wondering) > > Martin Klapetek wrote: > > There are still people around with a Solaris system. Not supported > doesn't mean we should actively remove existing support, IMHO. > > Is there any attempt to make sure KF5/PW2 is working with Solaris? > Because if the underlying components won't work on Solaris, there's no point > in keeping Solaris code in one tiny module sitting on top of the bigger > blocks. > > > it's fine to include "runtime" stuff in there. > > Cool, will add it then :)
I'm not sure Qt5 even compiles on Solaris, and it might be hard to convince Thiago to accept code for it, but I can ask what the status there is. The alternative is to have timeZone() and listTimeZones() calls in ktimezoned that wrap the Qt or Solaris calls. As for Qt monitoring the time zones: yes that would probably be the end result which is not ideal, but AFAIK we can't use a daemon inside Qt, or call dbus inside QtCore which rules out using ktimezoned if it's running, or systemd if it ever adds this feature. I have to post to the development list the design proposal for it and see if it gets shot down or not. - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review41816 ----------------------------------------------------------- On Oct. 18, 2013, 1 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113260/ > ----------------------------------------------------------- > > (Updated Oct. 18, 2013, 1 p.m.) > > > Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. > > > Repository: kde-runtime > > > Description > ------- > > Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out > that all the stuff KTZD was doing was added in QTimeZone, that includes > reading correct system files/env variables to obtain the timezone and > returning the proper system zone (KTZD did all this by itself). It also > doesn't need to parse the timezone files itself or watch for their changes as > QTimeZone objects are not stored. > > So now it's just a thin module watching /etc/timezone (used by Debian-based > distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian > in conjunction with /etc/timezone) for changes and if it detects a change, it > checks if the new timezone is really different and if it is, it sends out a > DBus signal "timeZoneChange". I changed it from "configChanged" as I think > "timeZoneChanged" makes way more sense. > > I didn't touch the Windows part as I have no way to test, would be nice if > someone could help with that. > > EDIT: I removed the other two DBus signals which were not used and I'm unsure > KTZD is the correct place for that now anyway. The only usage in > KSystemTimeZone can be replaced by own KDirWatch instance. > > > Diffs > ----- > > ktimezoned/ktimezoned.cpp f380c09 > ktimezoned/ktimezoned.h ff21807 > ktimezoned/CMakeLists.txt bafc85e > ktimezoned/ktimezoned_win.h 26e21cc > ktimezoned/ktimezoned_win.cpp cadfe3a > ktimezoned/ktimezonedbase.h ca00aca > ktimezoned/org.kde.KTimeZoned.xml daaa0b7 > > Diff: http://git.reviewboard.kde.org/r/113260/diff/ > > > Testing > ------- > > Tested by changing the timezone in different ways, change was detected and > signalled out. > > > Thanks, > > Martin Klapetek > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
