Review Request: Reset time format upon user request

David Faure faure at kde.org
Mon Dec 19 08:57:00 UTC 2011


On Sunday 18 December 2011 20:26:42 Lamarque V. Souza wrote:
> Em Sunday 18 December 2011, David Faure escreveu:
> > On Sunday 18 December 2011 12:51:25 Lamarque Vieira Souza wrote:
> > > Another problem with this approach is that we cannot prevent anybody
> > > else
> > > from listening to
> > > KGlobalSettings::self()->emitChange(KGlobalSettings::SettingsChanged,
> > > KGlobalSettings::SETTING_LOCALE).
> > 
> > What would be wrong with that? It would be the way to have a GUI that
> > reacts to changes in the locale settings. Every app and in particular
> > date/time widgets might want to listen to that and adapt.
> > Or did I misunderstand what this was about?
> 
> 	You misunderstood what I meant. You removed that paragraph from the
> context of the first paragraph of my last e-mail. What I meant is that if we
> use something like KLocale::commit() then we should not let others use
> KGlobalSettings::self()>emitChange(KGlobalSettings::SettingsChanged,
> KGlobalSettings::SETTING_LOCALE).

Minor issue. It would be harmless to signal a change when nothing changed.

The same is true for every other SETTING_* in there. Anyone can emit "the 
desktop path has changed", but if it wasn't changed, then nothing will happen 
anyway.

> 	The problem with KGlobalSettings::SETTING_LOCALE is that is solves only
> half the problem. I am using a hack to force the local KLocale instance in
> the clock plasmoid to reload the configuration files. There is no API in
> kdelibs to reload KLocale's configuration files and that is why I suggested
> using something like KLocale::commit() when Aaron complained about my
> second review of this patch.

Adding a commit() that is called after setFoo() calls, sounds good.

> > If everyone only went for the "minimal lines of code" fix all the time, we
> > would have one hell of a mess by now...
> 
> 	I am not familiar with this part of kdelibs and it is very clear it is a
> very sensible part of kdelibs. Anything wrong here will be noticed by
> everybody, that is why I am trying to do the minimum in this case.

This is what reviews are for.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5



More information about the Plasma-devel mailing list