Review Request 122400: Add timedated support into the clock KCM as an optional dependency

[Martin Gräßlin]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150209/a24a17c7/attachment-0001.html>