Re: New framework to review: KPackage

2014-12-10 Thread Marco Martin
On Wednesday 10 December 2014, Albert Astals Cid wrote: > > I would like to submit it (kpackage repo) for the usual 2 weeks period of > > review. > > Add const & > void setDefaultMimeTypes(QStringList mimeTypes); > void setMimeTypes(const char *key, QStringList mimeTypes); > > You probabl

Re: New framework to review: KPackage

2014-12-10 Thread David Edmundson
​The binary is called kpackagetool. Given the complications we've had with frameworks co-installability does it make sense to call it kpackagetool5? The class name in kpackagetool/kpackagetool.cpp should probably be renamed Documentation at the top of PackageLoader should avoid saying Plasma quit

Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread David Edmundson
> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote: > > Wasn't that part of the idea? Having it scale up the pixmap first when > > resizing and then re-rendering it later? > > Xuetian Weng wrote: > 1. icon size (the widget size) doesn't change frequently. Usually it only > happens when us

Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Sebastian Kügler
> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote: > > Wasn't that part of the idea? Having it scale up the pixmap first when > > resizing and then re-rendering it later? > > Xuetian Weng wrote: > 1. icon size (the widget size) doesn't change frequently. Usually it only > happens when us

Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Xuetian Weng
> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote: > > Wasn't that part of the idea? Having it scale up the pixmap first when > > resizing and then re-rendering it later? > > Xuetian Weng wrote: > 1. icon size (the widget size) doesn't change frequently. Usually it only > happens when us

Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread David Edmundson
> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote: > > Wasn't that part of the idea? Having it scale up the pixmap first when > > resizing and then re-rendering it later? > > Xuetian Weng wrote: > 1. icon size (the widget size) doesn't change frequently. Usually it only > happens when us

Re: Review Request 121090: Move kio-mtp to kio-extras

2014-12-10 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121090/#review71732 --- Ship it! Ship It! - David Edmundson On Dec. 8, 2014, 2:34

Re: Review Request 121090: Move kio-mtp to kio-extras

2014-12-10 Thread Jan Grulich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121090/ --- (Updated Dec. 10, 2014, 6:07 p.m.) Status -- This change has been ma

Re: Review Request 120388: Do not sync if wallet file does not exist

2014-12-10 Thread Arjun AK
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120388/#review71740 --- Ping. - Arjun AK On Sept. 27, 2014, 5:29 p.m., Arjun AK wro

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Mark Gaiser
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118452/#review71736 --- src/core/udsentry.cpp

OSX/CI: kio fails to build on branch master

2014-12-10 Thread Marko Käning
[ 23%] Built target udsentrybenchmark_automoc Scanning dependencies of target ktelnetservice5 Scanning dependencies of target lockingtest Cannot process input: '/Users/marko/WC/KDECI-builds/kio/build/src/ioslaves/http/kcookiejar/org.kde.KCookieServer.xml'. Stop. make[2]: *** [src/ioslaves/http/kc

Re: Review Request 121400: Add a new macro to create VERSIONINFO resources for Windows

2014-12-10 Thread Alex Merry
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121400/#review71745 --- What happens if you miss out an argument (ie: you put an empty

Re: Review Request 121400: Add a new macro to create VERSIONINFO resources for Windows

2014-12-10 Thread Nicolás Alvarez
> On Dec. 10, 2014, 5:38 p.m., Alex Merry wrote: > > What happens if you miss out an argument (ie: you put an empty string in > > the file)? Is that the same as not including the corresponding VALUE line > > in the file at all? > > > > Also, it would be nice to be able to optionally pass the v

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
> On Dec. 10, 2014, 7:40 p.m., Mark Gaiser wrote: > > src/core/udsentry.cpp, line 99 > > > > > > micro optimization, but d->fields[index] is - in my experience - > > slightly faster. nope, that is not true. >

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
On Dec. 10, 2014, 7:40 p.m., Frank Reininghaus wrote: > > As for all the appends that you do on a QVector, you might be able to speed > > them up significantly if you know the size (which you do). > > The reason for that potential speedup is append doing some stuff to > > determine if it can ap

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Mark Gaiser
On Dec. 10, 2014, 7:40 p.m., Frank Reininghaus wrote: > > As for all the appends that you do on a QVector, you might be able to speed > > them up significantly if you know the size (which you do). > > The reason for that potential speedup is append doing some stuff to > > determine if it can ap

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118452/#review71763 --- LGTM, some nitpicks. And of course we need the results of the

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
On Dec. 10, 2014, 7:40 p.m., Frank Reininghaus wrote: > > As for all the appends that you do on a QVector, you might be able to speed > > them up significantly if you know the size (which you do). > > The reason for that potential speedup is append doing some stuff to > > determine if it can ap

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
On Dec. 10, 2014, 7:40 p.m., Frank Reininghaus wrote: > > As for all the appends that you do on a QVector, you might be able to speed > > them up significantly if you know the size (which you do). > > The reason for that potential speedup is append doing some stuff to > > determine if it can ap

Re: Review Request 121400: Add a new macro to create VERSIONINFO resources for Windows

2014-12-10 Thread Alex Merry
> On Dec. 10, 2014, 8:38 p.m., Alex Merry wrote: > > What happens if you miss out an argument (ie: you put an empty string in > > the file)? Is that the same as not including the corresponding VALUE line > > in the file at all? > > > > Also, it would be nice to be able to optionally pass the v

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Mark Gaiser
On Dec. 10, 2014, 7:40 p.m., Frank Reininghaus wrote: > > As for all the appends that you do on a QVector, you might be able to speed > > them up significantly if you know the size (which you do). > > The reason for that potential speedup is append doing some stuff to > > determine if it can ap

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
On Dec. 10, 2014, 7:40 p.m., Frank Reininghaus wrote: > > As for all the appends that you do on a QVector, you might be able to speed > > them up significantly if you know the size (which you do). > > The reason for that potential speedup is append doing some stuff to > > determine if it can ap

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
On Dec. 10, 2014, 7:40 p.m., Frank Reininghaus wrote: > > As for all the appends that you do on a QVector, you might be able to speed > > them up significantly if you know the size (which you do). > > The reason for that potential speedup is append doing some stuff to > > determine if it can ap

Re: Review Request 121400: Add a new macro to create VERSIONINFO resources for Windows

2014-12-10 Thread Nicolás Alvarez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121400/ --- (Updated Dec. 10, 2014, 8:57 p.m.) Review request for Build System and KD

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
> On Dec. 10, 2014, 11:03 p.m., Milian Wolff wrote: > > LGTM, some nitpicks. And of course we need the results of the benchmark in > > release mode. NOTE: I just pushed some fixes to udsentrybenchmark, which was pretty broken in itself. It contained lots of unrelated stuff in its hotpath. Now,

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118452/#review71773 --- BTW: more food-for-thought is to use a `QVarLengthArray<*, 8>`

Re: Review Request 118452: Reduce the memory usage of UDSEntry by using QVector, rather than QHash, for the internal data storage

2014-12-10 Thread Milian Wolff
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118452/#review71774 --- src/core/udsentry.cpp

Re: Review Request 120388: Do not sync if wallet file does not exist

2014-12-10 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120388/#review71775 --- +1 Makes sense to me, still I'd like somebody more proficient

Re: Review Request 120388: Do not sync if wallet file does not exist

2014-12-10 Thread Arjun AK
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120388/ --- (Updated Dec. 11, 2014, 2:16 a.m.) Review request for KDE Frameworks, Àle

Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Xuetian Weng
> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote: > > Wasn't that part of the idea? Having it scale up the pixmap first when > > resizing and then re-rendering it later? > > Xuetian Weng wrote: > 1. icon size (the widget size) doesn't change frequently. Usually it only > happens when us

Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Xuetian Weng
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121411/ --- (Updated Dec. 11, 2014, 3:55 a.m.) Review request for KDE Frameworks, Pla