-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112099/#review37845
-----------------------------------------------------------


Thanks for the patch!

The enum thing seems a bit overkill to me, i mean, how "expensive" is the 
updateSample function after all? Personally, I'd just call it and that's it. 
Also makes sure you don't introduce subtle bugs in the future if someone 
expands a function to do something else but forgets to add an | enum

Anyone else has an opinion?

- Albert Astals Cid


On Aug. 15, 2013, 2:07 p.m., Santeri Piippo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112099/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2013, 2:07 p.m.)
> 
> 
> Review request for KDE Runtime and Albert Astals Cid.
> 
> 
> Description
> -------
> 
> when the input texts are changed the sample did not update and only was 
> updated with the 1 second timer. with applied patch it updates right away.
> 
> for performance reasons updateSample() was given a flags parameter for 
> filtering out what to update, defaults to all, i.e. original behavior.
> 
> 
> Diffs
> -----
> 
>   kcontrol/locale/kcmlocale.h c7b29bc 
>   kcontrol/locale/kcmlocale.cpp 62d5aef 
> 
> Diff: http://git.reviewboard.kde.org/r/112099/diff/
> 
> 
> Testing
> -------
> 
> replaced kcm_locale.so in kubuntu 13.04 with git-compiled one, works fine for 
> me.
> 
> 
> Thanks,
> 
> Santeri Piippo
> 
>

Reply via email to