Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review76740 --- Ship it! +1 from me - Lukáš Tinkl On Úno. 11, 2015, 3:51 odp., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Úno. 11, 2015, 3:51 odp.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 27, 2015, 7:02 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review76738 --- bump, or I'm going to push it anyway. - David Edmundson On Feb. 11, 2015, 2:51 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 11, 2015, 2:51 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 11, 2015, 2:51 p.m.) Review request for Plasma. Changes --- Don't do the stupid 5 second wait for ktimezoned if we're in timedated mode Add comments in bool KclockModule::timedatedSave() Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs (updated) - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 10, 2015, 7:51 p.m., Xuetian Weng wrote: kcms/dateandtime/main.cpp, line 146 https://git.reviewboard.kde.org/r/122400/diff/5/?file=348191#file348191line146 This part can be async. You can do it by chain two different callback if it need to be done one after another. Lambda would be helpful in this case to keep code short. David Edmundson wrote: I wrote on line 137 why I did this... but given lots of people keep commenting, but I'll change it and the legacy version too Note that on line 195 we block the entire UI for 5 seconds anyway, so changing this isn't going to have any noticable impact given that problem exists. Xuetian Weng wrote: so also check if (m_haveTimedated) for line 195? David Edmundson wrote: I didn't because we still use ktimezone in the UI for displaying... but I could just fix that too. New patch incoming. Update: We need to block save() finishing. Otherwise when you click OK the app will exit before the save finishes, and there's no sensible way to stop that. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75801 --- On Feb. 10, 2015, 3:55 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 10, 2015, 3:55 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 10, 2015, 3:55 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs (updated) - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 9, 2015, 7:47 a.m., Martin Gräßlin wrote: kcms/dateandtime/main.cpp, line 57 https://git.reviewboard.kde.org/r/122400/diff/4/?file=347911#file347911line57 does it need to be a blocking dbus call? Any chance in getting it use a watcher? It's tricky as load() is called by systemsettings externally, and I need to have loaded the UI before that happens... and I need to know the result of this in order to load the UI... - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75660 --- On Feb. 8, 2015, 5:18 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 8, 2015, 5:18 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/timedated1.xml PRE-CREATION kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 10, 2015, 7:51 p.m., Xuetian Weng wrote: kcms/dateandtime/main.cpp, line 146 https://git.reviewboard.kde.org/r/122400/diff/5/?file=348191#file348191line146 This part can be async. You can do it by chain two different callback if it need to be done one after another. Lambda would be helpful in this case to keep code short. David Edmundson wrote: I wrote on line 137 why I did this... but given lots of people keep commenting, but I'll change it and the legacy version too Note that on line 195 we block the entire UI for 5 seconds anyway, so changing this isn't going to have any noticable impact given that problem exists. Xuetian Weng wrote: so also check if (m_haveTimedated) for line 195? I didn't because we still use ktimezone in the UI for displaying... but I could just fix that too. New patch incoming. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75801 --- On Feb. 10, 2015, 3:55 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 10, 2015, 3:55 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 10, 2015, 7:51 p.m., Xuetian Weng wrote: kcms/dateandtime/main.cpp, line 146 https://git.reviewboard.kde.org/r/122400/diff/5/?file=348191#file348191line146 This part can be async. You can do it by chain two different callback if it need to be done one after another. Lambda would be helpful in this case to keep code short. David Edmundson wrote: I wrote on line 137 why I did this... but given lots of people keep commenting, but I'll change it and the legacy version too Note that on line 195 we block the entire UI for 5 seconds anyway, so changing this isn't going to have any noticable impact given that problem exists. so also check if (m_haveTimedated) for line 195? - Xuetian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75801 --- On Feb. 10, 2015, 3:55 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 10, 2015, 3:55 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 10, 2015, 7:51 p.m., Xuetian Weng wrote: kcms/dateandtime/main.cpp, line 146 https://git.reviewboard.kde.org/r/122400/diff/5/?file=348191#file348191line146 This part can be async. You can do it by chain two different callback if it need to be done one after another. Lambda would be helpful in this case to keep code short. I wrote on line 137 why I did this... but given lots of people keep commenting, but I'll change it and the legacy version too Note that on line 195 we block the entire UI for 5 seconds anyway, so changing this isn't going to have any noticable impact given that problem exists. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75801 --- On Feb. 10, 2015, 3:55 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 10, 2015, 3:55 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75801 --- kcms/dateandtime/main.cpp https://git.reviewboard.kde.org/r/122400/#comment52357 This part can be async. You can do it by chain two different callback if it need to be done one after another. Lambda would be helpful in this case to keep code short. - Xuetian Weng On Feb. 10, 2015, 3:55 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 10, 2015, 3:55 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 9, 2015, 7:47 a.m., Martin Gräßlin wrote: kcms/dateandtime/main.cpp, line 143 https://git.reviewboard.kde.org/r/122400/diff/4/?file=347911#file347911line143 why wait? If I understand correctly the next dbus calls do not depend on the outcome of the first one. So you could just fire all of them and only check in the end whether they failed. David Edmundson wrote: Yeah, that's sensible. I want the method to block because otherwise having that and the old kauth approach end up differing wildly; but only blocking once will be more sensible. It gives me a reason for removing the legacy system later :) Whoa! Turns out, no, I can't change this. If I do pendingSetNtpReply = iface.setNTP(false); pendingSetTimeReply = iface.setTime(time); pendingSetNtpReply.waitForFinished(); pendingSetTimeReply.waitForFinished(); I get an error back from timedated org.freedesktop.timedate1.AutomaticTimeSyncEnabled Automatic time synchronization is enabled Which means timedated must be handling each new DBus requests in a new thread something you really can't do in Qt. Somewhere between fascinating and mental. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75660 --- On Feb. 8, 2015, 5:18 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 8, 2015, 5:18 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/timedated1.xml PRE-CREATION kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 9, 2015, 7:47 a.m., Martin Gräßlin wrote: kcms/dateandtime/main.cpp, line 143 https://git.reviewboard.kde.org/r/122400/diff/4/?file=347911#file347911line143 why wait? If I understand correctly the next dbus calls do not depend on the outcome of the first one. So you could just fire all of them and only check in the end whether they failed. Yeah, that's sensible. I want the method to block because otherwise having that and the old kauth approach end up differing wildly; but only blocking once will be more sensible. It gives me a reason for removing the legacy system later :) On Feb. 9, 2015, 7:47 a.m., Martin Gräßlin wrote: kcms/dateandtime/main.cpp, lines 87-93 https://git.reviewboard.kde.org/r/122400/diff/4/?file=347911#file347911line87 suggestion: move it into the dbus reply check If your system DBus daemon is slow replying your system is more than slightly screwed anyway. I end up changing the main widget so it's not as trivial as just this line. I'll see. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75660 --- On Feb. 8, 2015, 5:18 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 8, 2015, 5:18 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/timedated1.xml PRE-CREATION kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 5, 2015, 5:16 p.m., Martin Gräßlin wrote: how difficult would it be to get this with runtime detection? Just because the distro compiles with systemd support doesn't mean the user uses systemd (c.f. Debian). David Edmundson wrote: They don't need to use it, they just need to have it installed. It's possible; we'd just have to query some read property from timedated and see if it errors in the kcmodule ctor then switch accordingly. Downside is it means we still needs to provide our helper, even though it won't be used and the code gets even uglier. Martin Gräßlin wrote: if it works with having it just installed without needing e.g. systemd as init, that's good enough, I'd say. I went for runtime. On reflection I think it makes sense. As much as I like stirring up some controversy, this is both politically and practically safer for now. It's not like people with timedated are going to lose much by having to install a tiny helper. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75487 --- On Feb. 8, 2015, 5:18 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 8, 2015, 5:18 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/timedated1.xml PRE-CREATION kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 8, 2015, 5:18 p.m.) Review request for Plasma. Changes --- Changed to runtime detection. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs (updated) - kcms/dateandtime/timedated1.xml PRE-CREATION kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75660 --- Thanks for going runtime selection. I think that will be appreciated kcms/dateandtime/CMakeLists.txt https://git.reviewboard.kde.org/r/122400/#comment52314 nitpick: removed an empty line kcms/dateandtime/dtime.cpp https://git.reviewboard.kde.org/r/122400/#comment52315 nitpick: added empty lines kcms/dateandtime/main.cpp https://git.reviewboard.kde.org/r/122400/#comment52317 does it need to be a blocking dbus call? Any chance in getting it use a watcher? kcms/dateandtime/main.cpp https://git.reviewboard.kde.org/r/122400/#comment52316 suggestion: move it into the dbus reply check kcms/dateandtime/main.cpp https://git.reviewboard.kde.org/r/122400/#comment52318 why wait? If I understand correctly the next dbus calls do not depend on the outcome of the first one. So you could just fire all of them and only check in the end whether they failed. - Martin Gräßlin On Feb. 8, 2015, 6:18 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 8, 2015, 6:18 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/timedated1.xml PRE-CREATION kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 5, 2015, 5:16 p.m., Martin Gräßlin wrote: how difficult would it be to get this with runtime detection? Just because the distro compiles with systemd support doesn't mean the user uses systemd (c.f. Debian). They don't need to use it, they just need to have it installed. It's possible; we'd just have to query some read property from timedated and see if it errors in the kcmodule ctor then switch accordingly. Downside is it means we still needs to provide our helper, even though it won't be used and the code gets even uglier. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75487 --- On Feb. 3, 2015, 11:44 a.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 11:44 a.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75487 --- how difficult would it be to get this with runtime detection? Just because the distro compiles with systemd support doesn't mean the user uses systemd (c.f. Debian). - Martin Gräßlin On Feb. 3, 2015, 12:44 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 12:44 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 5, 2015, 6:16 nachm., Martin Gräßlin wrote: how difficult would it be to get this with runtime detection? Just because the distro compiles with systemd support doesn't mean the user uses systemd (c.f. Debian). David Edmundson wrote: They don't need to use it, they just need to have it installed. It's possible; we'd just have to query some read property from timedated and see if it errors in the kcmodule ctor then switch accordingly. Downside is it means we still needs to provide our helper, even though it won't be used and the code gets even uglier. if it works with having it just installed without needing e.g. systemd as init, that's good enough, I'd say. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75487 --- On Feb. 3, 2015, 12:44 nachm., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 12:44 nachm.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 4, 2015, 3:13 p.m., Martin Gräßlin wrote: could the implementation be split in two files: one for legacy, one for systemd? I think this could make reading the file easier. Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? Bhushan Shah wrote: Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? there is systemd.pc so perhaps we can have something like FindSystemd.cmake? Bhushan Shah wrote: there is systemd.pc so perhaps we can have something like FindSystemd.cmake? Actually scratch that.. sorry for noise. I just realized why we can not have compile time check David Edmundson wrote: I don't think it can be split easily, we'd be splitting main. I deliberatley make it a cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. We could check for /usr/share/dbus-1/system-services... but arguably they could name it anything. actually I have #ifdefd half the file, I'll make two mains then. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75415 --- On Feb. 3, 2015, 11:44 a.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 11:44 a.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 4, 2015, 4:13 p.m., Martin Gräßlin wrote: could the implementation be split in two files: one for legacy, one for systemd? I think this could make reading the file easier. Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? Bhushan Shah wrote: Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? there is systemd.pc so perhaps we can have something like FindSystemd.cmake? Bhushan Shah wrote: there is systemd.pc so perhaps we can have something like FindSystemd.cmake? Actually scratch that.. sorry for noise. I just realized why we can not have compile time check David Edmundson wrote: I don't think it can be split easily, we'd be splitting main. I deliberatley make it a cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. We could check for /usr/share/dbus-1/system-services... but arguably they could name it anything. David Edmundson wrote: actually I have #ifdefd half the file, I'll make two mains then. We could check for /usr/share/dbus-1/system-services... but arguably they could name it anything. could be an idea for querying the default value of the option and adding some feature info. That would probably give more information especially to the BSDs. Right now with the option being default ON, they might be surprised that the datetime kcm breaks. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75415 --- On Feb. 3, 2015, 12:44 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 12:44 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75415 --- could the implementation be split in two files: one for legacy, one for systemd? I think this could make reading the file easier. Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? - Martin Gräßlin On Feb. 3, 2015, 12:44 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 12:44 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 4, 2015, 8:43 p.m., Martin Gräßlin wrote: could the implementation be split in two files: one for legacy, one for systemd? I think this could make reading the file easier. Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? there is systemd.pc so perhaps we can have something like FindSystemd.cmake? - Bhushan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75415 --- On Feb. 3, 2015, 5:14 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 5:14 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 4, 2015, 8:43 p.m., Martin Gräßlin wrote: could the implementation be split in two files: one for legacy, one for systemd? I think this could make reading the file easier. Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? Bhushan Shah wrote: Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? there is systemd.pc so perhaps we can have something like FindSystemd.cmake? there is systemd.pc so perhaps we can have something like FindSystemd.cmake? Actually scratch that.. sorry for noise. I just realized why we can not have compile time check - Bhushan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75415 --- On Feb. 3, 2015, 5:14 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 5:14 p.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 4, 2015, 3:13 p.m., Martin Gräßlin wrote: could the implementation be split in two files: one for legacy, one for systemd? I think this could make reading the file easier. Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? Bhushan Shah wrote: Is it possible to determine at compile time whether the systemd component is available? E.g. check whether a dbus file is installed at a known location? there is systemd.pc so perhaps we can have something like FindSystemd.cmake? Bhushan Shah wrote: there is systemd.pc so perhaps we can have something like FindSystemd.cmake? Actually scratch that.. sorry for noise. I just realized why we can not have compile time check I don't think it can be split easily, we'd be splitting main. I deliberatley make it a cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. We could check for /usr/share/dbus-1/system-services... but arguably they could name it anything. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75415 --- On Feb. 3, 2015, 11:44 a.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 11:44 a.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 11:43 a.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs (updated) - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 11:44 a.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs (updated) - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75273 --- Looks good kcms/dateandtime/dtime.cpp https://git.reviewboard.kde.org/r/122400/#comment52061 I guess this meant irrelevant? - Martin Klapetek On Feb. 3, 2015, 9:40 a.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 9:40 a.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122400: Add timedated support into the clock KCM as an optional dependency
On Feb. 3, 2015, 9:31 a.m., Martin Klapetek wrote: kcms/dateandtime/dtime.cpp, line 203 https://git.reviewboard.kde.org/r/122400/diff/1/?file=346550#file346550line203 I guess this meant irrelevant? yes, I accidentally a word. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/#review75273 --- On Feb. 3, 2015, 8:40 a.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122400/ --- (Updated Feb. 3, 2015, 8:40 a.m.) Review request for Plasma. Repository: plasma-desktop Description --- The current time setting helper is incredibly broken. It manually tries to run a range of NTP utilities, all of which are deprecated. We can just call timedated directly and cut out the middleman as it has uses polkit anyway. This is currently an optional dependency, and the original helper still exists. It makes the code messy, but we have users to support for now. Finding timedated is an cmake option rather than querying for systemd libs to make it easier for those deploying shims, such as BSD. (code is in two commits, first abstracting the saving from the dtime class; then adding in the second save mechanism) Diffs - kcms/dateandtime/CMakeLists.txt 4a987ae kcms/dateandtime/dateandtime.ui c073b5e kcms/dateandtime/dtime.h 1a90698 kcms/dateandtime/dtime.cpp 482e483 kcms/dateandtime/main.h c1e5234 kcms/dateandtime/main.cpp 0041a9d kcms/dateandtime/timedated1.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122400/diff/ Testing --- Thanks, David Edmundson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
