Re: Review Request 126742: Add removeAllActions() to remove every applet interface action.

2016-01-14 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126742/#review91089
---


+1 from me

though maybe clearActions fits Qt conventions a bit closer.

- David Edmundson


On Jan. 14, 2016, 12:42 p.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126742/
> ---
> 
> (Updated Jan. 14, 2016, 12:42 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Add removeAllActions() to remove every applet interface action. Is there an 
> equivalent way to do this in the plasmoid?
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/appletinterface.h 
> ad6849990789e0f960ff074e1a9db21ef64381ef 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 
> 8e4979ae9ec89da02f68e6438b5b0ab40a2495d0 
> 
> Diff: https://git.reviewboard.kde.org/r/126742/diff/
> 
> 
> Testing
> ---
> 
> Compile, runtime.
> 
> 
> Thanks,
> 
> James Smith
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126745: Fix karchive autotests on Windows

2016-01-14 Thread Kevin Funk

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126745/
---

Review request for KDE Frameworks.


Repository: karchive


Description
---

Still not perfect (no proper dependencies set), but better:
* Moves from CMake time to post-build time of kcompressiondevicetest
* Moves the test data to the actual output dir of kcompressiondevicetest

Still doesn't regenerate the data in case examples/ changes.


Diffs
-

  autotests/CMakeLists.txt 39881f90c614200a6c840b67d14e6c518f8e57e9 

Diff: https://git.reviewboard.kde.org/r/126745/diff/


Testing
---


Thanks,

Kevin Funk

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 1:22 p.m., andreas kainz wrote:
> > I have no problems to optimzie the svg files BUT my problem with the svg 
> > files is that they become more complex all the time last time the 
> > stylesheet, now optimization, 
> > 
> > I love scripts BUT it would be awesome for us to have this scripts before 
> > an distribution build there packages. In git keep it as simple as possible 
> > and when an distro will use the icons use therefore an script (check if the 
> > stylesheets fit, compress, make png files, )
> 
> Aleix Pol Gonzalez wrote:
> That's also the reason why I did it like this. This script doesn't need 
> to be run by the icon designer himself, it can be ran by the release manager 
> before creating the tarballs (or anyone else for that matter).
> 
> I don't think it's a problem having non-optimized svgs in git, but we 
> should provide users whatever is easier to execute when we release.
> 
> Sebastian Kügler wrote:
> If we run this only on releases, it means that typically no developer 
> uses the final result of the icons. (I run master all the time, it's usually 
> a pure coincidence if I run an actual release on my laptop. If the icons only 
> get compressed or optimized right before a release, that means that I've run 
> unoptimized svgs, different from what our users end up using, which may yield 
> different results.)
> 
> Aleix Pol Gonzalez wrote:
> Would you get something like scripty to run it?
> 
> Sebastian Kügler wrote:
> I've thought about it, I think it could work if it doesn't rewrite the 
> whole file every time it runs. (But only fixes 
> 'new problems', if introduced.)
> 
> Aleix Pol Gonzalez wrote:
> I'll investigate.

I just checked: optimizing an optimized files doesn't bear further changes.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91064
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126747: Drop use of deprecated Qt functions

2016-01-14 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126747/
---

Review request for KDE Frameworks.


Repository: polkit-qt-1


Description
---

Drop use of deprecated Qt functions


Diffs
-

  CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 
  agent/polkitqt1-agent-listener.cpp 38744ec6c0fbad833e22586cba0e602b413e2494 
  core/polkitqt1-authority.cpp efa8271bf72beda77294d8fcfe27b5e8ff6597be 

Diff: https://git.reviewboard.kde.org/r/126747/diff/


Testing
---

Compiles on Qt5 at least.


Thanks,

David Edmundson

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126746: Fix compilation with Qt5.6

2016-01-14 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126746/#review91098
---



CMakeLists.txt (line 36)


Can't this be -std=c++11 by now?


- Aleix Pol Gonzalez


On Jan. 14, 2016, 6:45 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126746/
> ---
> 
> (Updated Jan. 14, 2016, 6:45 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> Qt5.6 has c++0x in code in the headers, so it needs explicitly adding as a 
> compile flag when not using ECM.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 
> 
> Diff: https://git.reviewboard.kde.org/r/126746/diff/
> 
> 
> Testing
> ---
> 
> Compiles
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126746: Fix compilation with Qt5.6

2016-01-14 Thread David Edmundson


> On Jan. 14, 2016, 5:48 p.m., Aleix Pol Gonzalez wrote:
> > CMakeLists.txt, line 36
> > 
> >
> > Can't this be -std=c++11 by now?

It possibly could, but I opted for copying the same line as 
kde-modules/KDECompilerSettings.cmake


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126746/#review91098
---


On Jan. 14, 2016, 5:45 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126746/
> ---
> 
> (Updated Jan. 14, 2016, 5:45 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> Qt5.6 has c++0x in code in the headers, so it needs explicitly adding as a 
> compile flag when not using ECM.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 
> 
> Diff: https://git.reviewboard.kde.org/r/126746/diff/
> 
> 
> Testing
> ---
> 
> Compiles
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?

http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
Do not use QStringLiteral for empty strings


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 11:27 a.m., Marco Martin wrote:
> > does the svg stuff still workk?
> > this script goes well together the others i have (only in plasma-framework 
> > atm) to remove the hardcoded colors inkscape likes to put even when an 
> > element is using the stylesheet and the one to replace hardcoded colors 
> > with classes of the stylesheet.
> > 
> > 
> > as for compressing: i was actually thinking of uncompressing the svgs in 
> > plasma-framework :p (mainly because git)
> > 
> > what needs to happen is probably the compression to svgz to be done at 
> > build time and have all simple svgs in the repository
> 
> Aleix Pol Gonzalez wrote:
> What do you mean with "because git"? Because we might have the exact same 
> problem with svgo then.
> 
> Sebastian Kügler wrote:
> I'm doing an educated guess here: Marco wants readable diffs to icons, so 
> it's easier to spot if the coloring / stylesheets are still correct.
> 
> Aleix Pol Gonzalez wrote:
> If so, optmized svgs wouldn't work either, because they are squeezed in 
> one line.
> 
> Maybe it would be better to use different tooling to see the differences 
> on those cases?

Regarding gzip, which I understand it's a bit out of topic, I did the research 
too:

```
$ cat ~/frameworks/plasma-framework/.gitattributes 
*.svgz diff=gzip
$ cat .gitconfig
#...
[diff "gzip"]
textconv = gunzip -S .svgz -c
```

This lets you compare svgz files in git diff/show.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91055
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126742: Add removeAllActions() to remove every applet interface action.

2016-01-14 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126742/#review91095
---

Ship it!



src/scriptengines/qml/plasmoid/appletinterface.h (line 245)


Agree with David, clearActions()


- Marco Martin


On Jan. 14, 2016, 12:42 p.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126742/
> ---
> 
> (Updated Jan. 14, 2016, 12:42 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Add removeAllActions() to remove every applet interface action. Is there an 
> equivalent way to do this in the plasmoid?
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/appletinterface.h 
> ad6849990789e0f960ff074e1a9db21ef64381ef 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 
> 8e4979ae9ec89da02f68e6438b5b0ab40a2495d0 
> 
> Diff: https://git.reviewboard.kde.org/r/126742/diff/
> 
> 
> Testing
> ---
> 
> Compile, runtime.
> 
> 
> Thanks,
> 
> James Smith
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126742: Add removeAllActions() to remove every applet interface action.

2016-01-14 Thread Kai Uwe Broulik

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126742/#review91096
---



src/scriptengines/qml/plasmoid/appletinterface.cpp (line 504)


const QString & ?


- Kai Uwe Broulik


On Jan. 14, 2016, 12:42 nachm., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126742/
> ---
> 
> (Updated Jan. 14, 2016, 12:42 nachm.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Add removeAllActions() to remove every applet interface action. Is there an 
> equivalent way to do this in the plasmoid?
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/appletinterface.h 
> ad6849990789e0f960ff074e1a9db21ef64381ef 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 
> 8e4979ae9ec89da02f68e6438b5b0ab40a2495d0 
> 
> Diff: https://git.reviewboard.kde.org/r/126742/diff/
> 
> 
> Testing
> ---
> 
> Compile, runtime.
> 
> 
> Thanks,
> 
> James Smith
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 12:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings

afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
unit-tested, but there might be some code path I missed.
can you paste the actual warning it emitted, and also the .cpp file+line number 
?


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126746: Fix compilation with Qt5.6

2016-01-14 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126746/
---

Review request for KDE Frameworks.


Repository: polkit-qt-1


Description
---

Qt5.6 has c++0x in code in the headers, so it needs explicitly adding as a 
compile flag when not using ECM.


Diffs
-

  CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 

Diff: https://git.reviewboard.kde.org/r/126746/diff/


Testing
---

Compiles


Thanks,

David Edmundson

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: KIOGui ?

2016-01-14 Thread David Faure
On Thursday 14 January 2016 02:53:09 Friedrich W. H. Kossebau wrote:
> Am Mittwoch, 13. Januar 2016, 00:33:36 schrieb David Faure:
> > I'm about to write a class to handle favicons in a KIO library, rather than
> > using DBus communication to a (currently kded, could be kiod otherwise)
> > module.
> > 
> > I think there just isn't any point in using a central DBus module to handle
> > a shared cache when a lock file can do the job.
> > 
> > The question is: this would only depend on KIOCore and QImage. Shall I put
> > it in KIOWidgets or shall I create a new KIOGui library, for QML apps to
> > avoid the QWidget dependency ?
> 
> +1 for KIOGui.
> 
> Even if this means another module and thus increased complexity, it is still 
> good to start walking into the direction now where possible already.

It won't be another (git) module, it will be another library in the kio 
repository,
sitting between KIOCore and KIOWidgets.

> After all QtQuick-only apps are something KF5 wants to be attractive for as 
> well (think mobile platforms, SailfishOS even has that as requirement for 
> their appstore).
> 
> Adding such *Gui modules also starts to shape our minds more in that 
> direction, and gives positive feedback to those not postponing to split off 
> QWidget dependencies in the libs they work on, as they see others moving 
> forward there as well :)
> So when KF6 is around the corner where incompatible cleanup is possible, 
> going 
> QWidget-less where wanted should be easier to do if prepared now already.

OK, one more lib it is then.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/
---

(Updated Янв. 14, 2016, 8:52 д.п.)


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

fix QStringLiteral with multiple string initializers


Repository: kitemmodels


Description
---

Fixed most Clazy warnings in KItemModels.


Diffs (updated)
-

  autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
  autotests/kextracolumnsproxymodeltest.cpp 015ea65 
  autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
  autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
  autotests/kselectionproxymodeltest.cpp ac3d324 
  autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
  autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
  autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
  autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
  autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
  src/kconcatenaterowsproxymodel.cpp 074fcd3 
  src/kselectionproxymodel.cpp 4d7af29 
  tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
  tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
  tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
  tests/proxymodeltestapp/checkablewidget.cpp d51458a 
  tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
  tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
  tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
  tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
  tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
  tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
  tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 

Diff: https://git.reviewboard.kde.org/r/126735/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91055
---


does the svg stuff still workk?
this script goes well together the others i have (only in plasma-framework atm) 
to remove the hardcoded colors inkscape likes to put even when an element is 
using the stylesheet and the one to replace hardcoded colors with classes of 
the stylesheet.


as for compressing: i was actually thinking of uncompressing the svgs in 
plasma-framework :p (mainly because git)

what needs to happen is probably the compression to svgz to be done at build 
time and have all simple svgs in the repository

- Marco Martin


On Jan. 14, 2016, 3 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 3 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126723: Add wrapper for polkit_system_bus_name_get_user_sync

2016-01-14 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126723/
---

(Updated Jan. 14, 2016, 11:02 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, David Edmundson, Hrvoje Senjan, and Matthias 
Klumpp.


Changes
---

Submitted with commit d1bcbbe315dab07bef1ddb9c6f7335f88e0ebdbd by Martin 
Gräßlin to branch master.


Repository: polkit-qt-1


Description
---

This adds a new method to SystemBusNameSubject to get to the
UnixUserIdentity for the subject.

This feature depends on new functionality added in polkit 0.113, the
dependency is nevertheless not raised as the new API is backported in
distribution packages and we don't want to disrupt developer workflows
by depending on a version which is not available yet.


Diffs
-

  core/polkitqt1-subject.h 4c7a22bae51a501f9aed2eadac0579baea174d0d 
  core/polkitqt1-subject.cpp f0d69c647f00728525ee78e946a938f270ddb300 

Diff: https://git.reviewboard.kde.org/r/126723/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126725: prevent crash-on-exit in KSelectAction::~KSelectAction

2016-01-14 Thread René J . V . Bertin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126725/
---

(Updated Jan. 14, 2016, 12:21 p.m.)


Review request for KDE Frameworks and Christoph Feck.


Repository: kwidgetsaddons


Description
---

I was seeing a systematic crash on exitting kdenlive5, on OS X deep under the 
`delete menu()` instruction in `KSelectAction::~KSelectAction`. The backtrace 
suggested this might be due to a pending event (or an event due to the menu 
deletion) being delivered post-mortem to an instance of a QMenu related class.

My fix is based on 3 principles:
1) release the "foreign" member instance (`menu()`) before releasing the own 
d-ptr
2) Remove the QMenu instance from ourselves before deleting it to have one less 
potential dangling reference to it
3) QMenu is a QObject descendent that corresponds to a UI element: on OS X it 
is safer to dispose these through `deleteLater()` rather than directly.


Diffs (updated)
-

  src/kselectaction.cpp 1381099 

Diff: https://git.reviewboard.kde.org/r/126725/diff/


Testing
---

On OS X 10.9.5, Qt 5.5.1 and KF5 Frameworks 5.16.0, built for installation 
under /opt/local .

It seems that points 1) and 2) above already solve the crash issue in kdenlive 
on OS X, but the general principle stands so I suggest keeping 3) in library 
code like this.


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Reminder: Please add "Since" version info also to cmake macros and arguments

2016-01-14 Thread Friedrich W. H. Kossebau
Hi,

please remember to add some info about for which version of KF5 or ECM a new 
cmake macro or argument was added (or changed behaviour).

So people with a newer version of ECM/KF5 installed on their system, but 
working on a software using ECM/KF5 with lower minimal version requirements 
can quickly see whether some macro or parameter can be used or will break 
things for people with the minimal version installed.

As the API dox syntax for cmake macro seems to not have something like a 
"@since" tag, at least in one example I know instead the Since tag is simply 
written verbatim in the text, see
http://api.kde.org/ecm/module/ECMGenerateHeaders.html?highlight=Since

https://quickgit.kde.org/?p=extra-cmake-modules.git=blob=modules%2FECMGenerateHeaders.cmake

Or is there some better way now to tag the version where things appeared?


Example:
I just wanted to know since which KF5 version the cmake macro 
"kcoreaddons_add_plugin" exists, working on software with a KF5 5.7 minimal 
version requirement.
Having to hunt the version down in the git repo is not perfect ;) (and no, is 
too new to be used, added in 5.10)


I am happy to fix at least KF5CoreAddonsMacros.cmake now that I came across 
it, appending the Since behind the example, if that is the current syntax?

# Example:
#   kcoreaddons_add_plugin(kdeconnect_share JSON kdeconnect_share.json SOURCES 
${kdeconnect_share_SRCS})
#
# Since 5.10.0

Cheers
Friedrich
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/
---

Review request for KDE Frameworks and Aleix Pol Gonzalez.


Repository: kitemviews


Description
---

Fix Clazy warnings, except:
/src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
qDeleteAll(d->widgetInIndex.keys());


Diffs
-

  autotests/klistwidgetsearchlinetest.cpp b86d62c 
  src/kcategorizedview.cpp 185c24c 
  src/ktreewidgetsearchline.cpp b721c58 
  tests/kcategorizedviewtest.cpp 16dac9a 
  tests/ktreewidgetsearchlinetest.cpp adaf246 
  tests/kwidgetitemdelegatetest.cpp d4bba7a 

Diff: https://git.reviewboard.kde.org/r/126737/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126725: prevent crash-on-exit in KSelectAction::~KSelectAction

2016-01-14 Thread René J . V . Bertin


> On Jan. 12, 2016, 10:49 p.m., David Faure wrote:
> > Can I see the backtrace, with a description of how this gets triggered? I'm 
> > not sure the issue is fully understood (as shown by the use of "might" in 
> > the description...).
> > 
> > Is the action being destroyed by clicking into a submenu of the action? 
> > Otherwise I don't get the relation with the fix.
> 
> René J.V. Bertin wrote:
> A CrashReporter backtrace I still had logged:
> 
> ```
> Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
> Exception Codes: EXC_I386_GPFLT
> 
> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> 0   org.qt-project.QtCore 0x00010cbac886 
> QObject::property(char const*) const + 150 (qlist.h:105)
> 1   libqcocoa.dylib   0x00011189e37a 
> QCocoaMenuItem::~QCocoaMenuItem() + 58 (qcocoamenuitem.mm:109)
> 2   libqcocoa.dylib   0x00011189e52e 
> QCocoaMenuItem::~QCocoaMenuItem() + 14 (qcocoamenuitem.mm:106)
> 3   org.qt-project.QtWidgets  0x00010b2989bd 
> QMenu::actionEvent(QActionEvent*) + 749 (qmenu.cpp:3224)
> 4   org.qt-project.QtWidgets  0x00010b1643d0 
> QWidget::event(QEvent*) + 1232 (qwidget.cpp:9044)
> 5   org.qt-project.QtWidgets  0x00010b296bdc 
> QMenu::event(QEvent*) + 1164 (qmenu.cpp:2678)
> 6   org.qt-project.QtWidgets  0x00010b12653b 
> QApplicationPrivate::notify_helper(QObject*, QEvent*) + 251 
> (qapplication.cpp:3716)
> 7   org.qt-project.QtWidgets  0x00010b1298f4 
> QApplication::notify(QObject*, QEvent*) + 8212 (qapplication.cpp:3681)
> 8   org.qt-project.QtCore 0x00010cb7b433 
> QCoreApplication::notifyInternal(QObject*, QEvent*) + 115 (qthread_p.h:291)
> 9   org.qt-project.QtWidgets  0x00010b1582ed 
> QWidget::removeAction(QAction*) + 125 (qcoreapplication.h:224)
> 10  org.qt-project.QtWidgets  0x00010b11c711 
> QAction::~QAction() + 97 (qaction.cpp:566)
> 11  org.qt-project.QtWidgets  0x00010b16d16c 
> QWidgetAction::~QWidgetAction() + 444 (qwidgetaction.cpp:122)
> 12  libKF5WidgetsAddons.5.dylib   0x00010acddd80 
> KSelectAction::~KSelectAction() + 64 (kselectaction.cpp:99)
> 13  org.qt-project.QtCore 0x00010cba3d75 
> QObjectPrivate::deleteChildren() + 245 (qobject.cpp:1943)
> 14  org.qt-project.QtWidgets  0x00010b153ee1 
> QWidget::~QWidget() + 1441 (qwidget.cpp:1658)
> 150x0001093c16bb Bin::~Bin() 
> + 1067 (bin.cpp:456)
> 160x0001093c181e Bin::~Bin() 
> + 14 (bin.cpp:438)
> 170x000109443d3e 
> Core::~Core() + 62 (core.cpp:43)
> 180x00010940c95f 
> MainWindow::~MainWindow() + 207 (mainwindow.cpp:549)
> 190x00010940cd45 
> MainWindow::~MainWindow() + 21 (mainwindow.cpp:542)
> 20  org.qt-project.QtCore 0x00010cba4748 
> QObject::event(QEvent*) + 776 (qobject.cpp:4455)
> 21  org.qt-project.QtWidgets  0x00010b164d26 
> QWidget::event(QEvent*) + 3622 (qwidget.cpp:9105)
> 22  org.qt-project.QtWidgets  0x00010b26c45f 
> QMainWindow::event(QEvent*) + 911 (qmainwindow.cpp:1495)
> 23  libKF5XmlGui.5.dylib  0x00010a81e810 
> KMainWindow::event(QEvent*) + 624 (kmainwindow.cpp:829)
> 24  libKF5XmlGui.5.dylib  0x00010a856398 
> KXmlGuiWindow::event(QEvent*) + 24 (kxmlguiwindow.cpp:118)
> 25  org.qt-project.QtWidgets  0x00010b12653b 
> QApplicationPrivate::notify_helper(QObject*, QEvent*) + 251 
> (qapplication.cpp:3716)
> 26  org.qt-project.QtWidgets  0x00010b1298f4 
> QApplication::notify(QObject*, QEvent*) + 8212 (qapplication.cpp:3681)
> 27  org.qt-project.QtCore 0x00010cb7c0db 
> QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) + 971 
> (qthread_p.h:291)
> 28  org.qt-project.QtCore 0x00010cb7ba2c 
> QCoreApplication::exec() + 412 (qcoreapplication.cpp:1240)
> 290x0001093f3761 main + 10257 
> (main.cpp:118)
> 30  libdyld.dylib 0x7fff8508c5fd start + 1
> ```
> 
> When I say crash-on-exit, I mean that the crash occurs after selecting 
> the Quit menu item, or closing the main window.
> 
> René J.V. Bertin wrote:
> Here's a backtrace obtained in the debugger:
> 
> ```
> * thread #1: tid = 0x3727c3, 0x000103c4e886 
> QtCore`QObject::property(char const*) const [inlined] QListData::size() const 
> + 4 at qlist.h:105, queue = 'com.apple.main-thread', stop reason = 
> EXC_BAD_ACCESS 

Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Kåre Särs

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/#review91051
---



tests/proxymodeltestapp/proxyitemselectionwidget.cpp (line 47)


QStringLiteral with multiple string initializers does not work on Windows. 
All places without arguments could use QLatin1String.

There are multiple cases with the same issue in this review. Clazy should 
be fixed to not suggest this change...


- Kåre Särs


On Jan. 14, 2016, 12:24 a.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126735/
> ---
> 
> (Updated Jan. 14, 2016, 12:24 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> Fixed most Clazy warnings in KItemModels.
> 
> 
> Diffs
> -
> 
>   autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
>   autotests/kextracolumnsproxymodeltest.cpp 015ea65 
>   autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
>   autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
>   autotests/kselectionproxymodeltest.cpp ac3d324 
>   autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
>   autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
>   autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
>   autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
>   autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
>   src/kconcatenaterowsproxymodel.cpp 074fcd3 
>   src/kselectionproxymodel.cpp 4d7af29 
>   tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
>   tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
>   tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
>   tests/proxymodeltestapp/checkablewidget.cpp d51458a 
>   tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
>   tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
>   tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
>   tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
>   tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
>   tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
>   tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 
> 
> Diff: https://git.reviewboard.kde.org/r/126735/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Kåre Särs


> On Jan. 14, 2016, 8:47 a.m., Kåre Särs wrote:
> > tests/proxymodeltestapp/proxyitemselectionwidget.cpp, line 47
> > 
> >
> > QStringLiteral with multiple string initializers does not work on 
> > Windows. All places without arguments could use QLatin1String.
> > 
> > There are multiple cases with the same issue in this review. Clazy 
> > should be fixed to not suggest this change...

+1 Good to go from my point of view :) I'll let Aleix give the Shipit.


- Kåre


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/#review91051
---


On Jan. 14, 2016, 8:52 a.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126735/
> ---
> 
> (Updated Jan. 14, 2016, 8:52 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> Fixed most Clazy warnings in KItemModels.
> 
> 
> Diffs
> -
> 
>   autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
>   autotests/kextracolumnsproxymodeltest.cpp 015ea65 
>   autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
>   autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
>   autotests/kselectionproxymodeltest.cpp ac3d324 
>   autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
>   autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
>   autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
>   autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
>   autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
>   src/kconcatenaterowsproxymodel.cpp 074fcd3 
>   src/kselectionproxymodel.cpp 4d7af29 
>   tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
>   tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
>   tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
>   tests/proxymodeltestapp/checkablewidget.cpp d51458a 
>   tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
>   tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
>   tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
>   tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
>   tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
>   tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
>   tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 
> 
> Diff: https://git.reviewboard.kde.org/r/126735/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Making polkit-qt-1 a tier1 framework

2016-01-14 Thread Martin Gräßlin

Hi all,

I want to suggest to move polkit-qt-1 [1] from kdesupport to frameworks. 
Reasons are:


* kdesupport is basically what became tier1 in frameworks
* it's used by other frameworks, e.g. KAuth (tier 2) and in 
kde/workspace
* polkit-qt-1 is currently in a not really released state (last release 
in 2014, quite a few bugfixes around)


By moving it to frameworks we get closer to getting rid of kdesupport 
and get the making releases problem solved once and for all: bug fixes 
will get to users in a timely manner.


Opinions?

Cheers,
Martin

[1] git://anongit.kde.org/polkit-qt-1
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126742: Add removeAllActions() to remove every applet interface action.

2016-01-14 Thread James Smith

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126742/
---

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Add removeAllActions() to remove every applet interface action. Is there an 
equivalent way to do this in the plasmoid?


Diffs
-

  src/scriptengines/qml/plasmoid/appletinterface.h 
ad6849990789e0f960ff074e1a9db21ef64381ef 
  src/scriptengines/qml/plasmoid/appletinterface.cpp 
8e4979ae9ec89da02f68e6438b5b0ab40a2495d0 

Diff: https://git.reviewboard.kde.org/r/126742/diff/


Testing
---

Compile, runtime.


Thanks,

James Smith

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Sebastian Kügler


> On Jan. 14, 2016, 10:27 a.m., Marco Martin wrote:
> > does the svg stuff still workk?
> > this script goes well together the others i have (only in plasma-framework 
> > atm) to remove the hardcoded colors inkscape likes to put even when an 
> > element is using the stylesheet and the one to replace hardcoded colors 
> > with classes of the stylesheet.
> > 
> > 
> > as for compressing: i was actually thinking of uncompressing the svgs in 
> > plasma-framework :p (mainly because git)
> > 
> > what needs to happen is probably the compression to svgz to be done at 
> > build time and have all simple svgs in the repository
> 
> Aleix Pol Gonzalez wrote:
> What do you mean with "because git"? Because we might have the exact same 
> problem with svgo then.

I'm doing an educated guess here: Marco wants readable diffs to icons, so it's 
easier to spot if the coloring / stylesheets are still correct.


- Sebastian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91055
---


On Jan. 14, 2016, 3 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 3 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Making polkit-qt-1 a tier1 framework

2016-01-14 Thread Aleix Pol
On Thu, Jan 14, 2016 at 1:20 PM, Martin Gräßlin  wrote:
> Hi all,
>
> I want to suggest to move polkit-qt-1 [1] from kdesupport to frameworks.
> Reasons are:
>
> * kdesupport is basically what became tier1 in frameworks
> * it's used by other frameworks, e.g. KAuth (tier 2) and in kde/workspace
> * polkit-qt-1 is currently in a not really released state (last release in
> 2014, quite a few bugfixes around)
>
> By moving it to frameworks we get closer to getting rid of kdesupport and
> get the making releases problem solved once and for all: bug fixes will get
> to users in a timely manner.
>
> Opinions?
>
> Cheers,
> Martin
>
> [1] git://anongit.kde.org/polkit-qt-1

+1

Have you checked it passes the KDE Frameworks checklist?
https://community.kde.org/Frameworks/Policies
https://community.kde.org/Frameworks/CreationGuidelines

Aleix
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Sebastian Kügler


> On Jan. 14, 2016, 12:22 p.m., andreas kainz wrote:
> > I have no problems to optimzie the svg files BUT my problem with the svg 
> > files is that they become more complex all the time last time the 
> > stylesheet, now optimization, 
> > 
> > I love scripts BUT it would be awesome for us to have this scripts before 
> > an distribution build there packages. In git keep it as simple as possible 
> > and when an distro will use the icons use therefore an script (check if the 
> > stylesheets fit, compress, make png files, )
> 
> Aleix Pol Gonzalez wrote:
> That's also the reason why I did it like this. This script doesn't need 
> to be run by the icon designer himself, it can be ran by the release manager 
> before creating the tarballs (or anyone else for that matter).
> 
> I don't think it's a problem having non-optimized svgs in git, but we 
> should provide users whatever is easier to execute when we release.
> 
> Sebastian Kügler wrote:
> If we run this only on releases, it means that typically no developer 
> uses the final result of the icons. (I run master all the time, it's usually 
> a pure coincidence if I run an actual release on my laptop. If the icons only 
> get compressed or optimized right before a release, that means that I've run 
> unoptimized svgs, different from what our users end up using, which may yield 
> different results.)
> 
> Aleix Pol Gonzalez wrote:
> Would you get something like scripty to run it?

I've thought about it, I think it could work if it doesn't rewrite the whole 
file every time it runs. (But only fixes 
'new problems', if introduced.)


- Sebastian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91064
---


On Jan. 14, 2016, 3 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 3 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/#review91073
---

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Jan. 14, 2016, 9:52 a.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126735/
> ---
> 
> (Updated Jan. 14, 2016, 9:52 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> Fixed most Clazy warnings in KItemModels.
> 
> 
> Diffs
> -
> 
>   autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
>   autotests/kextracolumnsproxymodeltest.cpp 015ea65 
>   autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
>   autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
>   autotests/kselectionproxymodeltest.cpp ac3d324 
>   autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
>   autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
>   autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
>   autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
>   autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
>   src/kconcatenaterowsproxymodel.cpp 074fcd3 
>   src/kselectionproxymodel.cpp 4d7af29 
>   tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
>   tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
>   tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
>   tests/proxymodeltestapp/checkablewidget.cpp d51458a 
>   tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
>   tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
>   tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
>   tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
>   tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
>   tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
>   tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 
> 
> Diff: https://git.reviewboard.kde.org/r/126735/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 11:27 a.m., Marco Martin wrote:
> > does the svg stuff still workk?
> > this script goes well together the others i have (only in plasma-framework 
> > atm) to remove the hardcoded colors inkscape likes to put even when an 
> > element is using the stylesheet and the one to replace hardcoded colors 
> > with classes of the stylesheet.
> > 
> > 
> > as for compressing: i was actually thinking of uncompressing the svgs in 
> > plasma-framework :p (mainly because git)
> > 
> > what needs to happen is probably the compression to svgz to be done at 
> > build time and have all simple svgs in the repository

What do you mean with "because git"? Because we might have the exact same 
problem with svgo then.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91055
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread andreas kainz

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91064
---


I have no problems to optimzie the svg files BUT my problem with the svg files 
is that they become more complex all the time last time the stylesheet, now 
optimization, 

I love scripts BUT it would be awesome for us to have this scripts before an 
distribution build there packages. In git keep it as simple as possible and 
when an distro will use the icons use therefore an script (check if the 
stylesheets fit, compress, make png files, )

- andreas kainz


On Jan. 14, 2016, 3 vorm., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 3 vorm.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 5:48 a.m., Dirk Hohndel wrote:
> > I think this is a good idea - but just a small side note, this does not 
> > solve the problem that my patch addressed; the external references to 
> > linearGradients are not removed by svgo
> > 
> > /D

Good to know! Maybe we should ask them to include it...


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91046
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 1:22 p.m., andreas kainz wrote:
> > I have no problems to optimzie the svg files BUT my problem with the svg 
> > files is that they become more complex all the time last time the 
> > stylesheet, now optimization, 
> > 
> > I love scripts BUT it would be awesome for us to have this scripts before 
> > an distribution build there packages. In git keep it as simple as possible 
> > and when an distro will use the icons use therefore an script (check if the 
> > stylesheets fit, compress, make png files, )

That's also the reason why I did it like this. This script doesn't need to be 
run by the icon designer himself, it can be ran by the release manager before 
creating the tarballs (or anyone else for that matter).

I don't think it's a problem having non-optimized svgs in git, but we should 
provide users whatever is easier to execute when we release.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91064
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Making polkit-qt-1 a tier1 framework

2016-01-14 Thread Kevin Ottens
Hello,

On Thursday, 14 January 2016 13:20:01 CET Martin Gräßlin wrote:
> By moving it to frameworks we get closer to getting rid of kdesupport
> and get the making releases problem solved once and for all: bug fixes
> will get to users in a timely manner.
> 
> Opinions?

Personally I like it. Similar options for the rest of kdesupport should likely 
be considered as well.

Cheers.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com



signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 1:22 p.m., andreas kainz wrote:
> > I have no problems to optimzie the svg files BUT my problem with the svg 
> > files is that they become more complex all the time last time the 
> > stylesheet, now optimization, 
> > 
> > I love scripts BUT it would be awesome for us to have this scripts before 
> > an distribution build there packages. In git keep it as simple as possible 
> > and when an distro will use the icons use therefore an script (check if the 
> > stylesheets fit, compress, make png files, )
> 
> Aleix Pol Gonzalez wrote:
> That's also the reason why I did it like this. This script doesn't need 
> to be run by the icon designer himself, it can be ran by the release manager 
> before creating the tarballs (or anyone else for that matter).
> 
> I don't think it's a problem having non-optimized svgs in git, but we 
> should provide users whatever is easier to execute when we release.
> 
> Sebastian Kügler wrote:
> If we run this only on releases, it means that typically no developer 
> uses the final result of the icons. (I run master all the time, it's usually 
> a pure coincidence if I run an actual release on my laptop. If the icons only 
> get compressed or optimized right before a release, that means that I've run 
> unoptimized svgs, different from what our users end up using, which may yield 
> different results.)

Would you get something like scripty to run it?


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91064
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 1:03 p.m., Sergio Martins wrote:
> > Ship It!

Can you push it in two different commits ? One for the QStringLiteral change 
and another for the isEmpty()


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---

Ship it!


Ship It!

- Sergio Martins


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Making polkit-qt-1 a tier1 framework

2016-01-14 Thread Martin Bříza
On Thu, 14 Jan 2016 13:20:01 +0100, Martin Gräßlin   
wrote:



Hi all,

I want to suggest to move polkit-qt-1 [1] from kdesupport to frameworks.  
Reasons are:


* kdesupport is basically what became tier1 in frameworks
* it's used by other frameworks, e.g. KAuth (tier 2) and in kde/workspace
* polkit-qt-1 is currently in a not really released state (last release  
in 2014, quite a few bugfixes around)


By moving it to frameworks we get closer to getting rid of kdesupport  
and get the making releases problem solved once and for all: bug fixes  
will get to users in a timely manner.


Opinions?

Cheers,
Martin

[1] git://anongit.kde.org/polkit-qt-1


Hi,

I think the move is a good decision, especially considering the release  
cycle.


Martin
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Sebastian Kügler


> On Jan. 14, 2016, 12:22 p.m., andreas kainz wrote:
> > I have no problems to optimzie the svg files BUT my problem with the svg 
> > files is that they become more complex all the time last time the 
> > stylesheet, now optimization, 
> > 
> > I love scripts BUT it would be awesome for us to have this scripts before 
> > an distribution build there packages. In git keep it as simple as possible 
> > and when an distro will use the icons use therefore an script (check if the 
> > stylesheets fit, compress, make png files, )
> 
> Aleix Pol Gonzalez wrote:
> That's also the reason why I did it like this. This script doesn't need 
> to be run by the icon designer himself, it can be ran by the release manager 
> before creating the tarballs (or anyone else for that matter).
> 
> I don't think it's a problem having non-optimized svgs in git, but we 
> should provide users whatever is easier to execute when we release.

If we run this only on releases, it means that typically no developer uses the 
final result of the icons. (I run master all the time, it's usually a pure 
coincidence if I run an actual release on my laptop. If the icons only get 
compressed or optimized right before a release, that means that I've run 
unoptimized svgs, different from what our users end up using, which may yield 
different results.)


- Sebastian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91064
---


On Jan. 14, 2016, 3 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 3 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 1:22 p.m., andreas kainz wrote:
> > I have no problems to optimzie the svg files BUT my problem with the svg 
> > files is that they become more complex all the time last time the 
> > stylesheet, now optimization, 
> > 
> > I love scripts BUT it would be awesome for us to have this scripts before 
> > an distribution build there packages. In git keep it as simple as possible 
> > and when an distro will use the icons use therefore an script (check if the 
> > stylesheets fit, compress, make png files, )
> 
> Aleix Pol Gonzalez wrote:
> That's also the reason why I did it like this. This script doesn't need 
> to be run by the icon designer himself, it can be ran by the release manager 
> before creating the tarballs (or anyone else for that matter).
> 
> I don't think it's a problem having non-optimized svgs in git, but we 
> should provide users whatever is easier to execute when we release.
> 
> Sebastian Kügler wrote:
> If we run this only on releases, it means that typically no developer 
> uses the final result of the icons. (I run master all the time, it's usually 
> a pure coincidence if I run an actual release on my laptop. If the icons only 
> get compressed or optimized right before a release, that means that I've run 
> unoptimized svgs, different from what our users end up using, which may yield 
> different results.)
> 
> Aleix Pol Gonzalez wrote:
> Would you get something like scripty to run it?
> 
> Sebastian Kügler wrote:
> I've thought about it, I think it could work if it doesn't rewrite the 
> whole file every time it runs. (But only fixes 
> 'new problems', if introduced.)

I'll investigate.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91064
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


Looks good, I'll add Sergio, he maybe can explain what the warning meant in the 
first place.

Andrey, Are you sure you fixed all the warnings? I found some missing, for 
example:
`/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1: 
warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
[-Wclazy-missing-qobject]`

- Aleix Pol Gonzalez


On Jan. 14, 2016, 10:49 a.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 10:49 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 9:47 a.m., Kåre Särs wrote:
> > tests/proxymodeltestapp/proxyitemselectionwidget.cpp, line 47
> > 
> >
> > QStringLiteral with multiple string initializers does not work on 
> > Windows. All places without arguments could use QLatin1String.
> > 
> > There are multiple cases with the same issue in this review. Clazy 
> > should be fixed to not suggest this change...
> 
> Kåre Särs wrote:
> +1 Good to go from my point of view :) I'll let Aleix give the Shipit.

Actually it has, Andrey update your clazy ;).

```
commit 04a6c6665e73da087cec7d66039830d2932305f7
Author: Sergio Martins 
Date:   Thu Jan 14 10:23:06 2016 +

qstring-uneeded-heap-allocations: Rename msvc-compat to no-msvc-compat

Meaning msvc-compat is the default and if you don't want it you should:
export CLAZY_EXTRA_OPTIONS="qstring-uneeded-heap-allocations-no-msvc-compat"

Helps stopping annoying our windows friends.

Ignoring multi-string literals is not implemented yet, only ignoring list 
initialization.

CCBUG: 357966
```


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/#review91051
---


On Jan. 14, 2016, 9:52 a.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126735/
> ---
> 
> (Updated Jan. 14, 2016, 9:52 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> Fixed most Clazy warnings in KItemModels.
> 
> 
> Diffs
> -
> 
>   autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
>   autotests/kextracolumnsproxymodeltest.cpp 015ea65 
>   autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
>   autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
>   autotests/kselectionproxymodeltest.cpp ac3d324 
>   autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
>   autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
>   autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
>   autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
>   autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
>   src/kconcatenaterowsproxymodel.cpp 074fcd3 
>   src/kselectionproxymodel.cpp 4d7af29 
>   tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
>   tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
>   tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
>   tests/proxymodeltestapp/checkablewidget.cpp d51458a 
>   tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
>   tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
>   tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
>   tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
>   tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
>   tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
>   tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 
> 
> Diff: https://git.reviewboard.kde.org/r/126735/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126740: Add a script for optimizing svgs

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 11:27 a.m., Marco Martin wrote:
> > does the svg stuff still workk?
> > this script goes well together the others i have (only in plasma-framework 
> > atm) to remove the hardcoded colors inkscape likes to put even when an 
> > element is using the stylesheet and the one to replace hardcoded colors 
> > with classes of the stylesheet.
> > 
> > 
> > as for compressing: i was actually thinking of uncompressing the svgs in 
> > plasma-framework :p (mainly because git)
> > 
> > what needs to happen is probably the compression to svgz to be done at 
> > build time and have all simple svgs in the repository
> 
> Aleix Pol Gonzalez wrote:
> What do you mean with "because git"? Because we might have the exact same 
> problem with svgo then.
> 
> Sebastian Kügler wrote:
> I'm doing an educated guess here: Marco wants readable diffs to icons, so 
> it's easier to spot if the coloring / stylesheets are still correct.

If so, optmized svgs wouldn't work either, because they are squeezed in one 
line.

Maybe it would be better to use different tooling to see the differences on 
those cases?


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91055
---


On Jan. 14, 2016, 4 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 14, 2016, 4 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize-svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 1:03 p.m., Sergio Martins wrote:
> > Ship It!
> 
> Sergio Martins wrote:
> Can you push it in two different commits ? One for the QStringLiteral 
> change and another for the isEmpty()

Regarding the qDeleteAll(), the correct way is 
qDeleteAll(d->widgetInIndex.keyBegin(), d->widgetInIndex.keyEnd()), but that 
only works since Qt 5.6, so ignore this for now


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`

Strangely, I had not noticed the warning in the log . I get to the PC - see the 
log again.


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.

Some warnings , such as using qstringlitetal to initialize a blank line , I 
thought it wrong. Article Qt Weekly says that empty lines use the 
qstringliteral is not effective.


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2016-01-14 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/
---

(Updated Jan. 14, 2016, 2:59 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks, kwin and Albert Astals Cid.


Bugs: 354811
https://bugs.kde.org/show_bug.cgi?id=354811


Repository: kwindowsystem


Description
---

We observed that with Compiz as window manager various applications
crash if they use QGuiApplication instead of QApplication. The reason
for this is that on Compiz the mapViewport code paths are used and
that has a widgets dependency.

This change should ensure that applications do at least not crash
in this condition.

BUG: 354811


Diffs
-

  src/platforms/xcb/kwindowsystem.cpp 9d287043c24894ca3c29c439c7939b139da055e8 

Diff: https://git.reviewboard.kde.org/r/126397/diff/


Testing
---

User in referenced bug report tested it, works.


Thanks,

Martin Gräßlin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^
> 
> Andrey Cygankov wrote:
> I do not have this warning, which showed Aleix Pol.
> That log with warnings, the analyzer built from anongit on January 7
> https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0

When correcting warnings that indicated Aleix Pol(Insert Q_OBJECT macro in 
kwidgetitemdelegatepool.cpp, which contains class definition), assembly error: 
"AUTOGEN: error: 
/home/andrey/dev/projects/kitemviews/src/kwidgetitemdelegatepool.cpp: The file 
contains a Q_OBJECT macro, but does not include "kwidgetitemdelegatepool.moc" !"


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 1:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^
> 
> Andrey Cygankov wrote:
> I do not have this warning, which showed Aleix Pol.
> That log with warnings, the analyzer built from anongit on January 7
> https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0
> 
> Andrey Cygankov wrote:
> When correcting warnings that indicated Aleix Pol(Insert Q_OBJECT macro 
> in kwidgetitemdelegatepool.cpp, which contains class definition), assembly 
> error: "AUTOGEN: error: 
> /home/andrey/dev/projects/kitemviews/src/kwidgetitemdelegatepool.cpp: The 
> file contains a Q_OBJECT macro, but does not include 
> "kwidgetitemdelegatepool.moc" !"

In any case, you should pass QString() there, instead of "".


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 1:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 1:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 2:03 p.m., Sergio Martins wrote:
> > Ship It!
> 
> Sergio Martins wrote:
> Can you push it in two different commits ? One for the QStringLiteral 
> change and another for the isEmpty()
> 
> Sergio Martins wrote:
> Regarding the qDeleteAll(), the correct way is 
> qDeleteAll(d->widgetInIndex.keyBegin(), d->widgetInIndex.keyEnd()), but that 
> only works since Qt 5.6, so ignore this for now
> 
> Andrey Cygankov wrote:
> I do not have rights to commit.
> I think that it is not necessary to divide the RR into two parts. But I 
> have divided into two different diff, to make it convenient, here they are:
> 
> https://www.dropbox.com/s/tg1bhg71myv5nqb/kitemviews-clazyfix-tocommit.patch?dl=0
> 
> https://www.dropbox.com/s/j9chp5rx2cjkeay/kitemviews-isemptyfix-tocommit.patch?dl=0

While at it, can you request the commits right?
https://techbase.kde.org/Contribute/Get_a_Contributor_Account


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Jan. 14, 2016, 11:40 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 11:40 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126670: Fix some Clazy warnings in KPeople framework

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126670/
---

(Updated Jan. 14, 2016, 11:04 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Submitted with commit 7f896240d3be09e33d76a8ae8be953c498c662e7 by Aleix Pol on 
behalf of Andrey Cygankov to branch master.


Repository: kpeople


Description
---

Fix some Clazy warnings:
- qstring-uneeded-heap-allocations
- isempty-vs-count
- detaching-temporary
- variant-sanitizer


Diffs
-

  autotests/persondatatests.cpp 43a7c7b 
  autotests/personsmodeltest.cpp 1cbc3dc 
  examples/contactlistwidgets.cpp 56bb01c 
  examples/personwidget.cpp b4d73c8 
  src/personmanager.cpp 5c3d3e1 
  src/personpluginmanager.cpp f5396d9 
  src/widgets/actions.cpp ffaf16f 
  src/widgets/mergedelegate.cpp 01c988d 
  src/widgets/persondetailsview.cpp 7e4e24f 

Diff: https://git.reviewboard.kde.org/r/126670/diff/


Testing
---

Compiling without errors.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126742: Add clearActions() to remove every applet interface action.

2016-01-14 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126742/#review91122
---

Ship it!


do you have commit access?

- David Edmundson


On Jan. 15, 2016, 1:52 a.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126742/
> ---
> 
> (Updated Jan. 15, 2016, 1:52 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Add clearActions() to remove every applet interface action. Is there an 
> equivalent way to do this in the plasmoid?
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/appletinterface.h ad68499 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 8e4979a 
> 
> Diff: https://git.reviewboard.kde.org/r/126742/diff/
> 
> 
> Testing
> ---
> 
> Compile, runtime.
> 
> 
> Thanks,
> 
> James Smith
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126742: Add clearActions() to remove every applet interface action.

2016-01-14 Thread James Smith

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126742/
---

(Updated Jan. 15, 2016, 1:52 a.m.)


Review request for KDE Frameworks and Plasma.


Changes
---

removeAllActions() ->clearActions().


Summary (updated)
-

Add clearActions() to remove every applet interface action.


Repository: plasma-framework


Description (updated)
---

Add clearActions() to remove every applet interface action. Is there an 
equivalent way to do this in the plasmoid?


Diffs (updated)
-

  src/scriptengines/qml/plasmoid/appletinterface.h ad68499 
  src/scriptengines/qml/plasmoid/appletinterface.cpp 8e4979a 

Diff: https://git.reviewboard.kde.org/r/126742/diff/


Testing
---

Compile, runtime.


Thanks,

James Smith

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 1:03 п.п., Sergio Martins wrote:
> > Ship It!
> 
> Sergio Martins wrote:
> Can you push it in two different commits ? One for the QStringLiteral 
> change and another for the isEmpty()
> 
> Sergio Martins wrote:
> Regarding the qDeleteAll(), the correct way is 
> qDeleteAll(d->widgetInIndex.keyBegin(), d->widgetInIndex.keyEnd()), but that 
> only works since Qt 5.6, so ignore this for now

I do not have rights to commit.
I think that it is not necessary to divide the RR into two parts. But I have 
divided into two different diff, to make it convenient, here they are:
https://www.dropbox.com/s/tg1bhg71myv5nqb/kitemviews-clazyfix-tocommit.patch?dl=0
https://www.dropbox.com/s/j9chp5rx2cjkeay/kitemviews-isemptyfix-tocommit.patch?dl=0


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Янв. 14, 2016, 10:40 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 10:40 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 12:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^
> 
> Andrey Cygankov wrote:
> I do not have this warning, which showed Aleix Pol.
> That log with warnings, the analyzer built from anongit on January 7
> https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0
> 
> Andrey Cygankov wrote:
> When correcting warnings that indicated Aleix Pol(Insert Q_OBJECT macro 
> in kwidgetitemdelegatepool.cpp, which contains class definition), assembly 
> error: "AUTOGEN: error: 
> /home/andrey/dev/projects/kitemviews/src/kwidgetitemdelegatepool.cpp: The 
> file contains a Q_OBJECT macro, but does not include 
> "kwidgetitemdelegatepool.moc" !"
> 
> Aleix Pol Gonzalez wrote:
> In any case, you should pass QString() there, instead of "".

Those QString warnings seems correct to me. "" calls the QString(const char*) 
ctor, which we don't want.
Also note that clazy didn't suggest using QStringLiteral for those cases.
For the moc problem, you can include kwidgetitemdelegatepool.moc at the end of 
the .cpp file


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 10:40 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 10:40 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>


Re: Review Request 126736: Fix all Clazy warnings in KFileMetadata.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126736/
---

(Updated Jan. 14, 2016, 11:15 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Submitted with commit 27427ffebdd82141dc071423b6ef37c8a69119b0 by Aleix Pol on 
behalf of Andrey Cygankov to branch master.


Repository: kfilemetadata


Description
---

Fixed all Clazy warnings in KFileMetadata.


Diffs
-

  autotests/indexerextractortests.cpp d6dab41 
  autotests/odfextractortest.cpp bc25349 
  autotests/office2007extractortest.cpp e5170d2 
  src/extractorplugin.cpp cb907cd 
  src/extractors/odfextractor.cpp 78c4b37 
  src/extractors/office2007extractor.cpp 548763d 
  src/extractors/officeextractor.cpp efad5d4 
  tests/dump.cpp 6adb3a9 

Diff: https://git.reviewboard.kde.org/r/126736/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/
---

(Updated Jan. 14, 2016, 11:21 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.


Changes
---

Submitted with commit 69b721e73b421c1950ca32f1e774471060f2d4fe by Aleix Pol on 
behalf of Andrey Cygankov to branch master.


Repository: kitemviews


Description
---

Fix Clazy warnings, except:
/src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
qDeleteAll(d->widgetInIndex.keys());


Diffs
-

  autotests/klistwidgetsearchlinetest.cpp b86d62c 
  src/kcategorizedview.cpp 185c24c 
  src/ktreewidgetsearchline.cpp b721c58 
  tests/kcategorizedviewtest.cpp 16dac9a 
  tests/ktreewidgetsearchlinetest.cpp adaf246 
  tests/kwidgetitemdelegatetest.cpp d4bba7a 

Diff: https://git.reviewboard.kde.org/r/126737/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126750: Make KIconDialog and its sub-dialog Qt::WindowModal, rather than Qt::NonModal

2016-01-14 Thread Frank Reininghaus

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126750/
---

Review request for KDE Frameworks.


Bugs: 355310
https://bugs.kde.org/show_bug.cgi?id=355310


Repository: kiconthemes


Description
---

Currently, KIconDialog::showDialog() sets the modality to Qt::NonModal by 
calling

setModal(false);

I found that this was done in 
https://quickgit.kde.org/?p=kdelibs.git=commit=d0d2639c126f88a44c852b738550a9427c6260bb
 in order to prevent that a modal dialog locks an entire application, such as 
Plasma.

Unfortunately, this has an unwanted side effect in the "Places" of Dolphin and 
the file dialog: the KIconDialog is the child of a modal "Add Places Entry" 
dialog there. The KIconDialog itself works fine, but the "Browse..." sub 
dialog, which is a grand child of the modal dialog, is opened in the background 
and cannot be used (it could be that this worked for some reason in Qt4 times - 
I guess we would have received bug reports about this issue earlier otherwise).

This can be fixed by setting the modality to Qt::WindowModal, which ensures 
that the dialogs block their respective parents (but not the entire application 
- that would happen if the modality was set to Qt::ApplicationModal, for 
example by calling setModal(true)).

Note that there are two setModal(true) calls in the KIconDialog constructors. 
They have no effect if the dialog is opened with showDialog() (which is what 
happens if the dialog is opened by clicking a KIconButton) because the modality 
is overwritten there. I'm not sure though if there are any other uses of 
KIconDialog which would break if the apparently superfluous calls were removed. 
This might need further investigation.


Diffs
-

  src/kicondialog.cpp cca4ed3 

Diff: https://git.reviewboard.kde.org/r/126750/diff/


Testing
---

The "Browse..." sub dialog of the icon dialog works fine again in Dolphin and 
KWrite's file dialog when creating a new "Place".

I could not test if this affects Plasma somehow because I currently do not have 
a full self-built Plasma session running. It could probably be checked by 
opening the "Properties..." of a file in FolderView, clicking the icon, and 
then opening the "Browse..." sub dialog of the KIconDialog. This should 
hopefully not lock the entire Plasma session (because the dialogs are window 
modal, and not application modal). If anyone finds problems with that or has 
ideas for improvement, please let me know!


Thanks,

Frank Reininghaus

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/
---

(Updated Янв. 14, 2016, 10:40 п.п.)


Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.


Repository: kitemviews


Description
---

Fix Clazy warnings, except:
/src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
qDeleteAll(d->widgetInIndex.keys());


Diffs (updated)
-

  autotests/klistwidgetsearchlinetest.cpp b86d62c 
  src/kcategorizedview.cpp 185c24c 
  src/ktreewidgetsearchline.cpp b721c58 
  tests/kcategorizedviewtest.cpp 16dac9a 
  tests/ktreewidgetsearchlinetest.cpp adaf246 
  tests/kwidgetitemdelegatetest.cpp d4bba7a 

Diff: https://git.reviewboard.kde.org/r/126737/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:07 д.п., Aleix Pol Gonzalez wrote:
> > These look good. You wrote "most". Which warnings couldn't you fix?
> 
> Andrey Cygankov wrote:
> log file sent
> 
> Aleix Pol Gonzalez wrote:
> 
> `/home/andrey/dev/projects/kitemmodels/autotests/proxymodeltestsuite/proxymodeltest.h:762:17:
>  note: expanded from macro 'PROXYMODELTEST_MAIN'` << here you need to wrap in 
> QLatin1String().
> 
> `/home/andrey/dev/projects/kitemmodels/tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp:35:43:`
>  here you need to use the QStringLiteral.
> 
> `/home/andrey/dev/projects/kitemmodels/tests/proxymodeltestapp/selectioninqmlwidget.cpp:57`
>  also here: `quickView->setSource(QUrl::fromLocalFile(QStringLiteral(SRC_DIR 
> "/selection.qml")));`

I do not know how to correct the warnings out there that use a function defined 
by #define.
May be you give an example?


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/#review91040
---


On Янв. 14, 2016, 8:52 д.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126735/
> ---
> 
> (Updated Янв. 14, 2016, 8:52 д.п.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> Fixed most Clazy warnings in KItemModels.
> 
> 
> Diffs
> -
> 
>   autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
>   autotests/kextracolumnsproxymodeltest.cpp 015ea65 
>   autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
>   autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
>   autotests/kselectionproxymodeltest.cpp ac3d324 
>   autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
>   autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
>   autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
>   autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
>   autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
>   src/kconcatenaterowsproxymodel.cpp 074fcd3 
>   src/kselectionproxymodel.cpp 4d7af29 
>   tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
>   tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
>   tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
>   tests/proxymodeltestapp/checkablewidget.cpp d51458a 
>   tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
>   tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
>   tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
>   tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
>   tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
>   tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
>   tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 
> 
> Diff: https://git.reviewboard.kde.org/r/126735/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/
---

(Updated Jan. 14, 2016, 11:16 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Submitted with commit 1e487951342a79eff5c680d29c7e9ed852a47dc4 by Aleix Pol on 
behalf of Andrey Cygankov to branch master.


Repository: kitemmodels


Description
---

Fixed most Clazy warnings in KItemModels.


Diffs
-

  autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
  autotests/kextracolumnsproxymodeltest.cpp 015ea65 
  autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
  autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
  autotests/kselectionproxymodeltest.cpp ac3d324 
  autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
  autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
  autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
  autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
  autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
  src/kconcatenaterowsproxymodel.cpp 074fcd3 
  src/kselectionproxymodel.cpp 4d7af29 
  tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
  tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
  tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
  tests/proxymodeltestapp/checkablewidget.cpp d51458a 
  tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
  tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
  tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
  tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
  tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
  tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
  tests/proxymodeltestapp/selectioninqmlwidget.cpp 68e1d78 
  tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 

Diff: https://git.reviewboard.kde.org/r/126735/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^

I do not have this warning, which showed Aleix Pol.
That log with warnings, the analyzer built from anongit on January 7
https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126735: Fix most Clazy warnings in KItemModels.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126735/
---

(Updated Янв. 14, 2016, 11 п.п.)


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Repository: kitemmodels


Description
---

Fixed most Clazy warnings in KItemModels.


Diffs (updated)
-

  autotests/kconcatenaterowsproxymodeltest.cpp 0a2f794 
  autotests/kextracolumnsproxymodeltest.cpp 015ea65 
  autotests/krearrangecolumnsproxymodeltest.cpp 70e6ea9 
  autotests/krecursivefilterproxymodeltest.cpp 8c7c65e 
  autotests/kselectionproxymodeltest.cpp ac3d324 
  autotests/proxymodeltestsuite/dynamictreemodel.cpp d5a12e7 
  autotests/proxymodeltestsuite/dynamictreewidget.cpp 5a0462b 
  autotests/proxymodeltestsuite/modelcommander.cpp 8311e5b 
  autotests/proxymodeltestsuite/modelspy.cpp bb8e520 
  autotests/proxymodeltestsuite/proxymodeltest.cpp dd596c6 
  src/kconcatenaterowsproxymodel.cpp 074fcd3 
  src/kselectionproxymodel.cpp 4d7af29 
  tests/proxymodeltestapp/breadcrumbdirectionwidget.cpp 1e01ae3 
  tests/proxymodeltestapp/breadcrumbnavigationwidget.cpp a0493c2 
  tests/proxymodeltestapp/breadcrumbswidget.cpp d583856 
  tests/proxymodeltestapp/checkablewidget.cpp d51458a 
  tests/proxymodeltestapp/descendantpmwidget.cpp 39bd062 
  tests/proxymodeltestapp/kidentityproxymodelwidget.cpp 0150c70 
  tests/proxymodeltestapp/kreparentingproxymodel.cpp e79392c 
  tests/proxymodeltestapp/lessthanwidget.cpp 7af3c1c 
  tests/proxymodeltestapp/proxyitemselectionwidget.cpp e075dbf 
  tests/proxymodeltestapp/reparentingpmwidget.cpp 2593732 
  tests/proxymodeltestapp/selectioninqmlwidget.cpp 68e1d78 
  tests/proxymodeltestapp/selectionpmwidget.cpp f5b353a 

Diff: https://git.reviewboard.kde.org/r/126735/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126750: Make KIconDialog and its sub-dialog Qt::WindowModal, rather than Qt::NonModal

2016-01-14 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126750/#review91124
---


> The KIconDialog itself works fine, but the "Browse..." sub dialog, which is a 
> grand child of the modal dialog, is opened in the background and cannot be 
> used

this sounds like a Qt bug or a KFileDialog bug. The sub dialog should set a 
transient hint and that's respected by the window manager with and without 
having the modal flag.

My suggestion is that we verify it's a bug, locate it and fix it and don't work 
around it. To verify that it is a bug: xprop and xwininfo of all three windows 
are needed.

- Martin Gräßlin


On Jan. 15, 2016, 12:36 a.m., Frank Reininghaus wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126750/
> ---
> 
> (Updated Jan. 15, 2016, 12:36 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 355310
> https://bugs.kde.org/show_bug.cgi?id=355310
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Currently, KIconDialog::showDialog() sets the modality to Qt::NonModal by 
> calling
> 
> setModal(false);
> 
> I found that this was done in 
> https://quickgit.kde.org/?p=kdelibs.git=commit=d0d2639c126f88a44c852b738550a9427c6260bb
>  in order to prevent that a modal dialog locks an entire application, such as 
> Plasma.
> 
> Unfortunately, this has an unwanted side effect in the "Places" of Dolphin 
> and the file dialog: the KIconDialog is the child of a modal "Add Places 
> Entry" dialog there. The KIconDialog itself works fine, but the "Browse..." 
> sub dialog, which is a grand child of the modal dialog, is opened in the 
> background and cannot be used (it could be that this worked for some reason 
> in Qt4 times - I guess we would have received bug reports about this issue 
> earlier otherwise).
> 
> This can be fixed by setting the modality to Qt::WindowModal, which ensures 
> that the dialogs block their respective parents (but not the entire 
> application - that would happen if the modality was set to 
> Qt::ApplicationModal, for example by calling setModal(true)).
> 
> Note that there are two setModal(true) calls in the KIconDialog constructors. 
> They have no effect if the dialog is opened with showDialog() (which is what 
> happens if the dialog is opened by clicking a KIconButton) because the 
> modality is overwritten there. I'm not sure though if there are any other 
> uses of KIconDialog which would break if the apparently superfluous calls 
> were removed. This might need further investigation.
> 
> 
> Diffs
> -
> 
>   src/kicondialog.cpp cca4ed3 
> 
> Diff: https://git.reviewboard.kde.org/r/126750/diff/
> 
> 
> Testing
> ---
> 
> The "Browse..." sub dialog of the icon dialog works fine again in Dolphin and 
> KWrite's file dialog when creating a new "Place".
> 
> I could not test if this affects Plasma somehow because I currently do not 
> have a full self-built Plasma session running. It could probably be checked 
> by opening the "Properties..." of a file in FolderView, clicking the icon, 
> and then opening the "Browse..." sub dialog of the KIconDialog. This should 
> hopefully not lock the entire Plasma session (because the dialogs are window 
> modal, and not application modal). If anyone finds problems with that or has 
> ideas for improvement, please let me know!
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126746: Fix compilation with Qt5.6

2016-01-14 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126746/#review91112
---

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Jan. 14, 2016, 6:45 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126746/
> ---
> 
> (Updated Jan. 14, 2016, 6:45 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> Qt5.6 has c++0x in code in the headers, so it needs explicitly adding as a 
> compile flag when not using ECM.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 
> 
> Diff: https://git.reviewboard.kde.org/r/126746/diff/
> 
> 
> Testing
> ---
> 
> Compiles
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?

/tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << "" << 
"Shop");
^
/tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << "" << 
"Ground");
^
/tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
"Market");
^
/tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" << 
"Farmer");
   ^


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Making polkit-qt-1 a tier1 framework

2016-01-14 Thread Martin Gräßlin

Am 2016-01-14 18:40, schrieb David Edmundson:

On Thu, Jan 14, 2016 at 12:20 PM, Martin Gräßlin
 wrote:


Hi all,

I want to suggest to move polkit-qt-1 [1] from kdesupport to
frameworks. Reasons are:

* kdesupport is basically what became tier1 in frameworks
* it's used by other frameworks, e.g. KAuth (tier 2) and in
kde/workspace
* polkit-qt-1 is currently in a not really released state (last
release in 2014, quite a few bugfixes around)

By moving it to frameworks we get closer to getting rid of
kdesupport and get the making releases problem solved once and for
all: bug fixes will get to users in a timely manner.



Opinions?


Not whilst it does the weird "I can build Qt4 and 5 at the same time"
mess.


sure, that can (and should) be cleaned up. It's part of "need to get the 
CMake for frameworks" adjustment.


Cheers
Martin
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 12:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.

what do you mean by blank line, and can you link to that Qt weekly article ?


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel