Perşembe 28 May 2009 günü (saat 18:54:24) Aaron J. Seigo şunları yazmıştı: > On Thursday 28 May 2009, Gökçen Eraslan wrote: > > A patch for $subject is attached. This was the default behaviour in KDE3, > > now users have to open systemsettings to configure date/time. > > the "it was in KDE3" argument is meaningless. either a feature is good and > desirable, in which case it stands on its own and doesn't need such > justification, or it isn't a feature we want (for whatever reason(s)) and > then such justification doesn't matter either. >
I've said "it was in KDE3" because I see this as some kind of a usability
regression. I mean, if you can configure time by right clicking the clock
itself in KDE3 and now, you have to go through Menu->"System Settings"->"Date
and Time" path to setup time in KDE4, what do you call this? Improvement?
> this is very important because it changes how we think about features. if
> we think of things in terms of "what was in KDE3?" we'll end up replicating
> not just the good things in KDE3 but also the stupid things. and there were
> many stupid things in KDE3, as good as it was. (and yes, i was on occasion
> responsible for some of those stupid things too ;)
>
I didn't say, "we should do this just because it was in KDE3" or "let's put
all KDE3 features to KDE4", and I think we are smart enough to distinguish
what was a good feature and should be added to KDE4 and what was not.
> it's ok to use KDE3 as inspiration, just as it is ok to use MacOS, Windows,
> GNOME, BeOS, etc, etc as inspiration. but we don't say "This is the default
> behaviour in MacOS" either, do we? :)
>
> ok, enough philosophy.
>
> > Any reviews, comments?
>
> i wouldn't do it from init(), but inside of contextualActions(). this
> delays the creation of the object, which is good for both start up
> performance and memory usage (at least in the case where the user never
> pulls up a context menu).
OK, I'll change it.
>
> i also wouldn't bother with a createMenu method; it's 3 lines of code. just
> put it in contextualActions:
>
> if (m_actions.isEmpty()) {
> ... create the actions
> }
>
> also, this line:
>
> KToolInvocation::startServiceByDesktopName("clock", QStringList(), &error);
>
> is just asking for trouble :) it works now, but as soon as another item
> called "clock" appears we get some fun behaviour.
>
> instead, i'd suggest doing a query for the clock kcm from
> KServiceTypeTrader and start the returned service, if any.
>
Actually, this was the way I've seen in Pager applet's context menu. I'll
change this part of the patch also.
> > PS: By the way, there is a bug about time configuration. By default,
> > update interval of digital-clock plasmoid is 60 seconds when showSeconds
> > is false, and 1 second if it's true. If you change time from clock kcm
> > and showSeconds is false, digital-clock applet refreshes itself 1 minute
> > later. Maybe updateInterval should be hardcoded as 1 second regardless of
> > showSeconds variable.
>
> waking up 60x more often than necessary is a really good way to reduce
> battery life. the clock doesn't update once a minute just because it's lazy
> ;)
>
> as Daniel points out, the correct solution is a dbus signal (probably from
> kded4) that we can listen to.
Yes, you're right. Thanks for review :)
--
Gökçen Eraslan
Pardus Developer
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
