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

David Edmundson david at davidedmundson.co.uk
Mon Feb 9 14:58:37 UTC 2015



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

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


More information about the Plasma-devel mailing list