Review Request 112099: [kcontrol/locale] update the sample when input changed

Albert Astals Cid aacid at kde.org
Thu Aug 15 15:16:49 BST 2013


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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130815/29c99d5e/attachment.htm>


More information about the kde-core-devel mailing list