Re: Review Request 108992: Simple optimizations in SignalPlotter

2013-02-21 Thread Sebastian Kügler
Hi, On February 18th, 2013, 1:39 a.m. UTC, Aleix Pol Gonzalez wrote: I don't see loosening the variables' scope as a codebase improvement. Mostly otherwise. Also I'd like to know how you measured this 5% of improvement, which either way I'm unsure if it's worth it considering that this

Re: Review Request 109049: Fix favicon support for chrome bookmarks on krunner

2013-02-21 Thread Marco Gulino
On Feb. 20, 2013, 1:06 a.m., Àlex Fiestas wrote: I guess this will break compatibility with old versions then? if so, do you know from which version will it work? If this was not long ago, can we check the version and keep supporting the old and the new one? Àlex Fiestas wrote:

Re: Review Request 109049: Fix favicon support for chrome bookmarks on krunner

2013-02-21 Thread Marco Gulino
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109049/ --- (Updated Feb. 21, 2013, 7:29 p.m.) Review request for kde-workspace and

Re: Review Request 109049: Fix favicon support for chrome bookmarks on krunner

2013-02-21 Thread Àlex Fiestas
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109049/#review27876 --- Ship it! Tested the patch with chromiium 24.0.1312.70

Re: Review Request 108992: Simple optimizations in SignalPlotter

2013-02-21 Thread Aaron J. Seigo
On Feb. 18, 2013, 1:39 a.m., Aleix Pol Gonzalez wrote: I don't see loosening the variables' scope as a codebase improvement. Mostly otherwise. Also I'd like to know how you measured this 5% of improvement, which either way I'm unsure if it's worth it considering that this patch

Re: Review Request 109049: Fix favicon support for chrome bookmarks on krunner

2013-02-21 Thread Commit Hook
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109049/#review27881 --- This review has been submitted with commit

Re: Review Request 109049: Fix favicon support for chrome bookmarks on krunner

2013-02-21 Thread Marco Gulino
On Feb. 22, 2013, 12:41 a.m., Àlex Fiestas wrote: Tested the patch with chromiium 24.0.1312.70 (181759) worked fine. Code wise it looks fine as well. Thanks! Applied to branches KDE/4.10, and master, I guess it's enough. - Marco