Re: Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/#review28202 --- This review has been submitted with commit 7a42d977c90a5f0f387d170745e8a7b01f7d9675 by Kevin Kofler to branch KDE/4.10. - Commit Hook On Feb. 3, 2013, 4:30 a.m., Kevin Kofler wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/ --- (Updated Feb. 3, 2013, 4:30 a.m.) Review request for KDE Runtime and David Jarvie. Description --- /etc/localtime legitimately might not exist. The default is then UTC. But the file can then be created later, so watch for its creation. If we don't do this, when setting the time zone for the first time using kcm_clock, the initially set time zone will fail to get reloaded and the dialog will unexpectedly jump back to UTC. This problem shows up on Fedora 18, see: https://bugzilla.redhat.com/show_bug.cgi?id=906972 Please note that to test the fix with kcm_clock, you also need the kcm_clock (kde-workspace) fix from: https://git.reviewboard.kde.org/r/108711/ (which is already approved and which I'll push to KDE/4.10 and merge to master as soon as the 4.10.0 tagging freeze is lifted). Diffs - ktimezoned/ktimezoned.cpp 4eafa4e Diff: http://git.reviewboard.kde.org/r/108727/diff/ Testing --- Builds against at least 4.10.0 and 4.9.5. Works at runtime (and appears to fix the bug): https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5 Thanks, Kevin Kofler
Re: Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.
On Feb. 27, 2013, 1:07 p.m., Commit Hook wrote: This review has been submitted with commit 7a42d977c90a5f0f387d170745e8a7b01f7d9675 by Kevin Kofler to branch KDE/4.10. Merged to master with merge commit c36f1809671d434db1be700ef9c433f40a9157b5. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/#review28202 --- On Feb. 3, 2013, 4:30 a.m., Kevin Kofler wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/ --- (Updated Feb. 3, 2013, 4:30 a.m.) Review request for KDE Runtime and David Jarvie. Description --- /etc/localtime legitimately might not exist. The default is then UTC. But the file can then be created later, so watch for its creation. If we don't do this, when setting the time zone for the first time using kcm_clock, the initially set time zone will fail to get reloaded and the dialog will unexpectedly jump back to UTC. This problem shows up on Fedora 18, see: https://bugzilla.redhat.com/show_bug.cgi?id=906972 Please note that to test the fix with kcm_clock, you also need the kcm_clock (kde-workspace) fix from: https://git.reviewboard.kde.org/r/108711/ (which is already approved and which I'll push to KDE/4.10 and merge to master as soon as the 4.10.0 tagging freeze is lifted). Diffs - ktimezoned/ktimezoned.cpp 4eafa4e Diff: http://git.reviewboard.kde.org/r/108727/diff/ Testing --- Builds against at least 4.10.0 and 4.9.5. Works at runtime (and appears to fix the bug): https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5 Thanks, Kevin Kofler
Re: Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/#review27845 --- Ship it! I'm sorry this has taken so long - I've been incredibly busy recently. The patch looks fine. The only thing which occurs to me is to wonder whether the same scenario could happen with /etc/timezone (used by BSD) - if so, that file's creation should really be checked for as well as /etc/localtime. Without knowing whether that is actually possible, I think it might be worth adding the following comment where the patch sets mLocalIdFile: If under BSD it is possible for /etc/localtime to be missing but created later, we should also watch for its creation. - David Jarvie On Feb. 3, 2013, 4:30 a.m., Kevin Kofler wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/ --- (Updated Feb. 3, 2013, 4:30 a.m.) Review request for KDE Runtime and David Jarvie. Description --- /etc/localtime legitimately might not exist. The default is then UTC. But the file can then be created later, so watch for its creation. If we don't do this, when setting the time zone for the first time using kcm_clock, the initially set time zone will fail to get reloaded and the dialog will unexpectedly jump back to UTC. This problem shows up on Fedora 18, see: https://bugzilla.redhat.com/show_bug.cgi?id=906972 Please note that to test the fix with kcm_clock, you also need the kcm_clock (kde-workspace) fix from: https://git.reviewboard.kde.org/r/108711/ (which is already approved and which I'll push to KDE/4.10 and merge to master as soon as the 4.10.0 tagging freeze is lifted). Diffs - ktimezoned/ktimezoned.cpp 4eafa4e Diff: http://git.reviewboard.kde.org/r/108727/diff/ Testing --- Builds against at least 4.10.0 and 4.9.5. Works at runtime (and appears to fix the bug): https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5 Thanks, Kevin Kofler
Re: Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.
On Feb. 5, 2013, 8:27 p.m., David Faure wrote: Looks ok to me, although I don't really know this code. So, should I wait for David Jarvie or some other person who feels familiar with this code to give it a go, or do you think I can commit this right now? - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/#review26711 --- On Feb. 3, 2013, 4:30 a.m., Kevin Kofler wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/ --- (Updated Feb. 3, 2013, 4:30 a.m.) Review request for KDE Runtime and David Jarvie. Description --- /etc/localtime legitimately might not exist. The default is then UTC. But the file can then be created later, so watch for its creation. If we don't do this, when setting the time zone for the first time using kcm_clock, the initially set time zone will fail to get reloaded and the dialog will unexpectedly jump back to UTC. This problem shows up on Fedora 18, see: https://bugzilla.redhat.com/show_bug.cgi?id=906972 Please note that to test the fix with kcm_clock, you also need the kcm_clock (kde-workspace) fix from: https://git.reviewboard.kde.org/r/108711/ (which is already approved and which I'll push to KDE/4.10 and merge to master as soon as the 4.10.0 tagging freeze is lifted). Diffs - ktimezoned/ktimezoned.cpp 4eafa4e Diff: http://git.reviewboard.kde.org/r/108727/diff/ Testing --- Builds against at least 4.10.0 and 4.9.5. Works at runtime (and appears to fix the bug): https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5 Thanks, Kevin Kofler
Re: Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/#review26711 --- Looks ok to me, although I don't really know this code. - David Faure On Feb. 3, 2013, 4:30 a.m., Kevin Kofler wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/ --- (Updated Feb. 3, 2013, 4:30 a.m.) Review request for KDE Runtime and David Jarvie. Description --- /etc/localtime legitimately might not exist. The default is then UTC. But the file can then be created later, so watch for its creation. If we don't do this, when setting the time zone for the first time using kcm_clock, the initially set time zone will fail to get reloaded and the dialog will unexpectedly jump back to UTC. This problem shows up on Fedora 18, see: https://bugzilla.redhat.com/show_bug.cgi?id=906972 Please note that to test the fix with kcm_clock, you also need the kcm_clock (kde-workspace) fix from: https://git.reviewboard.kde.org/r/108711/ (which is already approved and which I'll push to KDE/4.10 and merge to master as soon as the 4.10.0 tagging freeze is lifted). Diffs - ktimezoned/ktimezoned.cpp 4eafa4e Diff: http://git.reviewboard.kde.org/r/108727/diff/ Testing --- Builds against at least 4.10.0 and 4.9.5. Works at runtime (and appears to fix the bug): https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5 Thanks, Kevin Kofler
Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108727/ --- Review request for KDE Runtime and David Jarvie. Description --- /etc/localtime legitimately might not exist. The default is then UTC. But the file can then be created later, so watch for its creation. If we don't do this, when setting the time zone for the first time using kcm_clock, the initially set time zone will fail to get reloaded and the dialog will unexpectedly jump back to UTC. This problem shows up on Fedora 18, see: https://bugzilla.redhat.com/show_bug.cgi?id=906972 Please note that to test the fix with kcm_clock, you also need the kcm_clock (kde-workspace) fix from: https://git.reviewboard.kde.org/r/108711/ (which is already approved and which I'll push to KDE/4.10 and merge to master as soon as the 4.10.0 tagging freeze is lifted). Diffs - ktimezoned/ktimezoned.cpp 4eafa4e Diff: http://git.reviewboard.kde.org/r/108727/diff/ Testing --- Builds against at least 4.10.0 and 4.9.5. Works at runtime (and appears to fix the bug): https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5 Thanks, Kevin Kofler