[Differential] [Updated, 609 lines] D2854: New: ECMGenerateApiDox, for generating qch & tag files

[kossebau (Friedrich W. H. Kossebau)]

kossebau updated this revision to Diff 6941.
kossebau added a comment.


  Fixing value of exported ${name}_APIDOX_TAGSFILE & tiny improvements

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2854?vs=6927=6941

BRANCH
  addGenerateApiDox

REVISION DETAIL
  https://phabricator.kde.org/D2854

AFFECTED FILES
  modules/ECMDoxygenQCH.config.in
  modules/ECMGenerateApiDox.cmake

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kossebau
Cc: staniek, winterz, ochurlaud, #kdevelop, #frameworks

Re: Review Request 128852: In listview mode use the default implemention of moveCursor

[David Edmundson]

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

(Updated Sept. 27, 2016, 12:14 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kitemviews


Description
---

KCategorizedView overrides the moveCursor to handle moving the cursor in
the grid; however it's own implementation doesn't handle key up/down
when it's not in an exact grid mode; which happens to also include list
mode.

In this case we don't need anything custom and the QListView
implementation works perfectly.

This fixes KPluginSelector keyboard navigation.


Diffs
-

  src/kcategorizedview.cpp 7e60a38060dc18fbe546495cfb0fdd64ec5b95d1 

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


Testing
---


Thanks,

David Edmundson

Review Request 129002: Use QCDebug in KIdleTime

[David Edmundson]

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

Review request for KDE Frameworks.


Repository: kidletime


Description
---

Also move some warnings to QCWarning() and reactivated some commented
out debug messages now that they're not on by default


Diffs
-

  src/plugins/xsync/CMakeLists.txt 46175f717672739d6f24f0b6831b119b2b86e07d 
  src/plugins/xsync/xsyncbasedpoller.cpp 
05c29eb8fb185eefd25907b7e77bd6b87e9cccb0 

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


Testing
---


Thanks,

David Edmundson

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Aleix Pol Gonzalez]

> On Sept. 26, 2016, 4:25 p.m., Aleix Pol Gonzalez wrote:
> > Again, I'll refer to how it's done in KCoreAddons. I don't think it's 
> > really useful to need to define variables for every framework you are 
> > compiling.
> 
> Luigi Toscano wrote:
> Is this last sentence part of another conversation that I missed?
> 
> Ralf Habacker wrote:
> This patch does it the same way as already implemented in kcoreaddons 
> https://github.com/KDE/kcoreaddons/blob/master/KF5CoreAddonsConfig.cmake.in#L11.
> 
> Aleix Pol Gonzalez wrote:
> The part I was referring to is actually:
> ```
> if(CMAKE_CROSSCOMPILING AND KF5_HOST_TOOLING)
> find_file(TARGETSFILE KF5CoreAddons/KF5CoreAddonsToolingTargets.cmake 
> PATHS ${KF5_HOST_TOOLING} ${CMAKE_CURRENT_LIST_DIR} NO_DEFAULT_PATH)
> include("${TARGETSFILE}")
> else()
> ```
> 
> I don't think we should take into account CROSSCOMPILING if 
> KF5_HOST_TOOLING isn't specified. Including the target files should define 
> all the needed targets.
> 
> Ralf Habacker wrote:
> @alex: You remember that I asked about 8 month ago about an example how 
> to implement this way and did not get any answer ? Any now you insist to use 
> a broken way without any help or tutorial ?
> 
> Aleix Pol Gonzalez wrote:
> My apologies if I haven't been verbose enough. I can try to document 
> `KF5_HOST_TOOLING` later tonight. Would that help?
> 
> My name is Aleix.
> 
> Ralf Habacker wrote:
> @aleix: From my experience on adding cross compile support at 
> https://build.opensuse.org/project/monitor/home:rhabacker:branches:windows:mingw:win32:KF526,
>  which is complete and works, there are several details to cover and I'm not 
> sure how much work remains to make it complete using the KF5_HOST_TOOLING way 
> after you documentated it.  Because the original KDE cmake guru abandoned his 
> approach to implement cross compile support with KDE 4.5 and never resumes it 
> looks really to have some major issues. 
> 
> My suggestion is therefore to complete the recent approach (which can 
> coexist with the incomplete KF5_HOST_TOOLING stuff if wished) to have a 
> stable and working solution and then to see how to improve cross compile 
> support by supporting the KF5_HOST_TOOLING way.
> 
> In short my approach compiles required native cross tools at  
> home:rhabacker:branches:KDE:Frameworks526 (kcoreaddons and sonnet also 
> includes required tools for cross compiling but related kf5 packages version 
> 5.26 already installs them unconditionally in the development package. (With 
> a few pending kf5 build system related patches reviewed this project can be 
> skipped). 
> 
> At 
> https://build.opensuse.org/project/monitor/home:rhabacker:branches:windows:mingw:win32:KF526
>  all required native tools are packaged (and relocated to 
> /usr/bin/-) by the mingw32-cross-kf5-tools package.
> 
> With this and other mingw32-cross... packages provided by the opensuse 
> windows:mingw:win32 project, windows kf5 packages are build. The mingw32-kxx 
> packages fetches kf5 source packages extracted directly from kde git server 
> using tags and uses local patches from for fixing remaining issues (for 
> example 
> https://build.opensuse.org/package/show/home:rhabacker:branches:windows:mingw:win32:KF526/mingw32-kdoctools).
> 
> BTW: Sorry for using wrong name.

Whatever, what can I say?
If you ask me, it looks like an ad-hoc solution to get your thing working 
there, but if that will bear any positive results then go for it.

+0 from me.


- Aleix


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


On Sept. 26, 2016, 4:23 p.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 p.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 129019: KFileItemActions: add addPluginActionsTo(QMenu *).

[David Faure]

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

(Updated Sept. 27, 2016, 12:26 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit dffe7e1e60b59fd241a50f9d222f0c0a2ef6e305 by David Faure 
to branch master.


Repository: kio


Description
---

This was missing in KFileItemActions so apps had to implement
this by themselves (this particular implementation is inspired by Dolphin's,
while KonqPopupMenu only had support for .desktop files and not JSON metadata).

This API will also be useful in folderview (which I'm porting away
from KonqPopupMenu) and kfind (same strategy).


Diffs
-

  src/widgets/kfileitemactions.h 9b025549efdceef7f32dc461f3d5bfa2ee10d938 
  src/widgets/kfileitemactions.cpp b31da0a38bfef634b0be207be9a87c9ba6864230 
  src/widgets/kfileitemactions_p.h 3f669feb3cf54b61dcb7ad0a22bc005210e18899 

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


Testing
---

Porting dolphin, folderview and kfind to this. Using KIO version checks of 
course.


Thanks,

David Faure

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Ralf Habacker]

> On Sept. 26, 2016, 4:25 nachm., Aleix Pol Gonzalez wrote:
> > Again, I'll refer to how it's done in KCoreAddons. I don't think it's 
> > really useful to need to define variables for every framework you are 
> > compiling.
> 
> Luigi Toscano wrote:
> Is this last sentence part of another conversation that I missed?
> 
> Ralf Habacker wrote:
> This patch does it the same way as already implemented in kcoreaddons 
> https://github.com/KDE/kcoreaddons/blob/master/KF5CoreAddonsConfig.cmake.in#L11.
> 
> Aleix Pol Gonzalez wrote:
> The part I was referring to is actually:
> ```
> if(CMAKE_CROSSCOMPILING AND KF5_HOST_TOOLING)
> find_file(TARGETSFILE KF5CoreAddons/KF5CoreAddonsToolingTargets.cmake 
> PATHS ${KF5_HOST_TOOLING} ${CMAKE_CURRENT_LIST_DIR} NO_DEFAULT_PATH)
> include("${TARGETSFILE}")
> else()
> ```
> 
> I don't think we should take into account CROSSCOMPILING if 
> KF5_HOST_TOOLING isn't specified. Including the target files should define 
> all the needed targets.
> 
> Ralf Habacker wrote:
> @alex: You remember that I asked about 8 month ago about an example how 
> to implement this way and did not get any answer ? Any now you insist to use 
> a broken way without any help or tutorial ?
> 
> Aleix Pol Gonzalez wrote:
> My apologies if I haven't been verbose enough. I can try to document 
> `KF5_HOST_TOOLING` later tonight. Would that help?
> 
> My name is Aleix.

@aleix: From my experience on adding cross compile support at 
https://build.opensuse.org/project/monitor/home:rhabacker:branches:windows:mingw:win32:KF526,
 which is complete and works, there are several details to cover and I'm not 
sure how much work remains to make it complete using the KF5_HOST_TOOLING way 
after you documentated it.  Because the original KDE cmake guru abandoned his 
approach to implement cross compile support with KDE 4.5 and never resumes it 
looks really to have some major issues. 

My suggestion is therefore to complete the recent approach (which can coexist 
with the incomplete KF5_HOST_TOOLING stuff if wished) to have a stable and 
working solution and then to see how to improve cross compile support by 
supporting the KF5_HOST_TOOLING way.

In short my approach compiles required native cross tools at  
home:rhabacker:branches:KDE:Frameworks526 (kcoreaddons and sonnet also includes 
required tools for cross compiling but related kf5 packages version 5.26 
already installs them unconditionally in the development package. (With a few 
pending kf5 build system related patches reviewed this project can be skipped). 

At 
https://build.opensuse.org/project/monitor/home:rhabacker:branches:windows:mingw:win32:KF526
 all required native tools are packaged (and relocated to 
/usr/bin/-) by the mingw32-cross-kf5-tools package.

With this and other mingw32-cross... packages provided by the opensuse 
windows:mingw:win32 project, windows kf5 packages are build. The mingw32-kxx 
packages fetches kf5 source packages extracted directly from kde git server 
using tags and uses local patches from for fixing remaining issues (for example 
https://build.opensuse.org/package/show/home:rhabacker:branches:windows:mingw:win32:KF526/mingw32-kdoctools).

BTW: Sorry for using wrong name.


- Ralf


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


On Sept. 26, 2016, 4:23 nachm., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 nachm.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 129032: Don't 'inline' public functions to avoid ABI breakage.

[Aleix Pol Gonzalez]

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



+1 makes sense

- Aleix Pol Gonzalez


On Sept. 26, 2016, 11:41 p.m., José Manuel  Santamaría Lema wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129032/
> ---
> 
> (Updated Sept. 26, 2016, 11:41 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Don't 'inline' public functions to avoid ABI breakage.
> 
> 
> Diffs
> -
> 
>   src/widgets/kpropertiesdialog.h a85037a 
>   src/widgets/kpropertiesdialog.cpp 5f64478 
> 
> Diff: https://git.reviewboard.kde.org/r/129032/diff/
> 
> 
> Testing
> ---
> 
> When packaging kio for kubuntu we realized there was a couple of missing 
> symbols which are a couple of deprecated functions, these functions were 
> "inlined" (without using the 'inline' keyword). 
> This is the offending commit:
> https://quickgit.kde.org/?p=kio.git=commitdiff=b36d368f8004d949597fbe9dc83d6b70418c22f8
> 
> From the binary compatibility page "Do's and Don'ts":
> https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts
> You cannot...
> [...]
> For existing functions of any type:
> [...]
> inline it (this includes moving a member function's body to the class 
> definition, even without the inline keyword).
> 
> The proposed patch moves the functions implementation from the .h file to the 
> .cpp file so this way the binary compatibility is kept.
> 
> 
> Thanks,
> 
> José Manuel  Santamaría Lema
> 
>

Re: Review Request 129032: Don't 'inline' public functions to avoid ABI breakage.

[José Manuel Santamaría Lema]

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

(Updated Sept. 26, 2016, 11:41 p.m.)


Review request for KDE Frameworks and David Faure.


Repository: kio


Description
---

Don't 'inline' public functions to avoid ABI breakage.


Diffs
-

  src/widgets/kpropertiesdialog.h a85037a 
  src/widgets/kpropertiesdialog.cpp 5f64478 

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


Testing
---

When packaging kio for kubuntu we realized there was a couple of missing 
symbols which are a couple of deprecated functions, these functions were 
"inlined" (without using the 'inline' keyword). 
This is the offending commit:
https://quickgit.kde.org/?p=kio.git=commitdiff=b36d368f8004d949597fbe9dc83d6b70418c22f8

>From the binary compatibility page "Do's and Don'ts":
https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts
You cannot...
[...]
For existing functions of any type:
[...]
inline it (this includes moving a member function's body to the class 
definition, even without the inline keyword).

The proposed patch moves the functions implementation from the .h file to the 
.cpp file so this way the binary compatibility is kept.


Thanks,

José Manuel  Santamaría Lema

Re: Review Request 129019: KFileItemActions: add addPluginActionsTo(QMenu *).

[Elvis Angelaccio]

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


Ship it!




One less reason for Dolphin to have its own context menu class ;)

- Elvis Angelaccio


On Sept. 26, 2016, 7:58 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129019/
> ---
> 
> (Updated Sept. 26, 2016, 7:58 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This was missing in KFileItemActions so apps had to implement
> this by themselves (this particular implementation is inspired by Dolphin's,
> while KonqPopupMenu only had support for .desktop files and not JSON 
> metadata).
> 
> This API will also be useful in folderview (which I'm porting away
> from KonqPopupMenu) and kfind (same strategy).
> 
> 
> Diffs
> -
> 
>   src/widgets/kfileitemactions.h 9b025549efdceef7f32dc461f3d5bfa2ee10d938 
>   src/widgets/kfileitemactions.cpp b31da0a38bfef634b0be207be9a87c9ba6864230 
>   src/widgets/kfileitemactions_p.h 3f669feb3cf54b61dcb7ad0a22bc005210e18899 
> 
> Diff: https://git.reviewboard.kde.org/r/129019/diff/
> 
> 
> Testing
> ---
> 
> Porting dolphin, folderview and kfind to this. Using KIO version checks of 
> course.
> 
> 
> Thanks,
> 
> David Faure
> 
>

Re: Review Request 129019: KFileItemActions: add addPluginActionsTo(QMenu *).

[David Faure]

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

(Updated Sept. 26, 2016, 7:58 p.m.)


Review request for KDE Frameworks.


Changes
---

now with the right patch


Repository: kio


Description
---

This was missing in KFileItemActions so apps had to implement
this by themselves (this particular implementation is inspired by Dolphin's,
while KonqPopupMenu only had support for .desktop files and not JSON metadata).

This API will also be useful in folderview (which I'm porting away
from KonqPopupMenu) and kfind (same strategy).


Diffs (updated)
-

  src/widgets/kfileitemactions.h 9b025549efdceef7f32dc461f3d5bfa2ee10d938 
  src/widgets/kfileitemactions.cpp b31da0a38bfef634b0be207be9a87c9ba6864230 
  src/widgets/kfileitemactions_p.h 3f669feb3cf54b61dcb7ad0a22bc005210e18899 

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


Testing
---

Porting dolphin, folderview and kfind to this. Using KIO version checks of 
course.


Thanks,

David Faure

Re: Review Request 129019: KFileItemActions: add addPluginActionsTo(QMenu *).

[David Faure]

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

(Updated Sept. 26, 2016, 7:55 p.m.)


Review request for KDE Frameworks.


Changes
---

Made both suggested fixes.


Repository: kio


Description
---

This was missing in KFileItemActions so apps had to implement
this by themselves (this particular implementation is inspired by Dolphin's,
while KonqPopupMenu only had support for .desktop files and not JSON metadata).

This API will also be useful in folderview (which I'm porting away
from KonqPopupMenu) and kfind (same strategy).


Diffs (updated)
-

  src/kiod/kiod_main.cpp 345f46d498a70cadee50ee356d6e0f366476ba92 

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


Testing (updated)
---

Porting dolphin, folderview and kfind to this. Using KIO version checks of 
course.


Thanks,

David Faure

Re: Review Request 129019: KFileItemActions: add addPluginActionsTo(QMenu *).

[David Faure]

> On Sept. 26, 2016, 9:53 a.m., Elvis Angelaccio wrote:
> > src/widgets/kfileitemactions.cpp, line 493
> > 
> >
> > There should be a `addedPlugins.append(jsonMetadata.pluginId());` after 
> > this line. For example I have ark installed both from distro package and in 
> > my own install prefix, so I get the ark actions twice.

Thanks for the test !

I debugged my duplicates, it was another issue: ark switching from a 
servicemenu desktop file to a plugin. Since I use "make install", I still had 
both in my install prefix, so both would appear. The historical solution to 
such issues was to install a ark_servicemenu.desktop file with Hidden=true in 
it, to overwrite the old file. Only useful to people who install from sources 
of course, binary packages don't need this.
Anyhow after a bit of install dir cleanup it works fine. I added your 
suggestion though, of course, for the issue you saw.


- David


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


On Sept. 26, 2016, 8:10 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129019/
> ---
> 
> (Updated Sept. 26, 2016, 8:10 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This was missing in KFileItemActions so apps had to implement
> this by themselves (this particular implementation is inspired by Dolphin's,
> while KonqPopupMenu only had support for .desktop files and not JSON 
> metadata).
> 
> This API will also be useful in folderview (which I'm porting away
> from KonqPopupMenu) and kfind (same strategy).
> 
> 
> Diffs
> -
> 
>   src/widgets/kfileitemactions.h 9b025549efdceef7f32dc461f3d5bfa2ee10d938 
>   src/widgets/kfileitemactions.cpp b31da0a38bfef634b0be207be9a87c9ba6864230 
>   src/widgets/kfileitemactions_p.h 3f669feb3cf54b61dcb7ad0a22bc005210e18899 
> 
> Diff: https://git.reviewboard.kde.org/r/129019/diff/
> 
> 
> Testing
> ---
> 
> Porting dolphin, folderview and kfind to this.
> 
> Still debugging why some service menu entries appear duplicated though - 
> might or might not be related to this.
> 
> 
> Thanks,
> 
> David Faure
> 
>

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Aleix Pol Gonzalez]

> On Sept. 26, 2016, 4:25 p.m., Aleix Pol Gonzalez wrote:
> > Again, I'll refer to how it's done in KCoreAddons. I don't think it's 
> > really useful to need to define variables for every framework you are 
> > compiling.
> 
> Luigi Toscano wrote:
> Is this last sentence part of another conversation that I missed?
> 
> Ralf Habacker wrote:
> This patch does it the same way as already implemented in kcoreaddons 
> https://github.com/KDE/kcoreaddons/blob/master/KF5CoreAddonsConfig.cmake.in#L11.
> 
> Aleix Pol Gonzalez wrote:
> The part I was referring to is actually:
> ```
> if(CMAKE_CROSSCOMPILING AND KF5_HOST_TOOLING)
> find_file(TARGETSFILE KF5CoreAddons/KF5CoreAddonsToolingTargets.cmake 
> PATHS ${KF5_HOST_TOOLING} ${CMAKE_CURRENT_LIST_DIR} NO_DEFAULT_PATH)
> include("${TARGETSFILE}")
> else()
> ```
> 
> I don't think we should take into account CROSSCOMPILING if 
> KF5_HOST_TOOLING isn't specified. Including the target files should define 
> all the needed targets.
> 
> Ralf Habacker wrote:
> @alex: You remember that I asked about 8 month ago about an example how 
> to implement this way and did not get any answer ? Any now you insist to use 
> a broken way without any help or tutorial ?

My apologies if I haven't been verbose enough. I can try to document 
`KF5_HOST_TOOLING` later tonight. Would that help?

My name is Aleix.


- Aleix


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


On Sept. 26, 2016, 4:23 p.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 p.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

[Differential] [Updated] D2854: New: ECMGenerateApiDox, for generating qch & tag files

[kossebau (Friedrich W. H. Kossebau)]

kossebau updated the summary for this revision.

REVISION DETAIL
  https://phabricator.kde.org/D2854

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kossebau
Cc: staniek, winterz, ochurlaud, #kdevelop, #frameworks

[Differential] [Updated, 603 lines] D2854: New: ECMGenerateApiDox, for generating qch & tag files

[kossebau (Friedrich W. H. Kossebau)]

kossebau updated this revision to Diff 6927.
kossebau added a comment.


  Fix generated inter-qch links, add support for unversioned namespace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2854?vs=6917=6927

BRANCH
  addGenerateApiDox

REVISION DETAIL
  https://phabricator.kde.org/D2854

AFFECTED FILES
  modules/ECMDoxygenQCH.config.in
  modules/ECMGenerateApiDox.cmake

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kossebau
Cc: staniek, winterz, ochurlaud, #kdevelop, #frameworks

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Ralf Habacker]

> On Sept. 26, 2016, 4:25 nachm., Aleix Pol Gonzalez wrote:
> > Again, I'll refer to how it's done in KCoreAddons. I don't think it's 
> > really useful to need to define variables for every framework you are 
> > compiling.
> 
> Luigi Toscano wrote:
> Is this last sentence part of another conversation that I missed?
> 
> Ralf Habacker wrote:
> This patch does it the same way as already implemented in kcoreaddons 
> https://github.com/KDE/kcoreaddons/blob/master/KF5CoreAddonsConfig.cmake.in#L11.
> 
> Aleix Pol Gonzalez wrote:
> The part I was referring to is actually:
> ```
> if(CMAKE_CROSSCOMPILING AND KF5_HOST_TOOLING)
> find_file(TARGETSFILE KF5CoreAddons/KF5CoreAddonsToolingTargets.cmake 
> PATHS ${KF5_HOST_TOOLING} ${CMAKE_CURRENT_LIST_DIR} NO_DEFAULT_PATH)
> include("${TARGETSFILE}")
> else()
> ```
> 
> I don't think we should take into account CROSSCOMPILING if 
> KF5_HOST_TOOLING isn't specified. Including the target files should define 
> all the needed targets.

@alex: You remember that I asked about 8 month ago about an example how to 
implement this way and did not get any answer ? Any now you insist to use a 
broken way without any help or tutorial ?


- Ralf


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


On Sept. 26, 2016, 4:23 nachm., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 nachm.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Jenkins-kde-ci: breeze-icons master stable-kf5-qt5 » Linux,gcc - Build # 297 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/297/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 26 Sep 2016 15:19:32 +
Build duration: 3 min 26 sec

CHANGE SET
Revision 506f17347b06955effb2429216bbd22798e52189 by sitter: (concat at runtime)
  change: edit autotests/dupetest.cpp
  change: edit autotests/testdata.h.cmake


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 
test(s)Failed: TestSuite.dupe

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 82/132 
(62%)CONDITIONAL 35/78 (45%)

By packages
  
default>
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 
14/26 (54%)
autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 40/75 (53%)CONDITIONAL 
21/52 (40%)

Jenkins-kde-ci: breeze-icons master kf5-qt5 » Linux,gcc - Build # 299 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/299/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 26 Sep 2016 15:19:32 +
Build duration: 2 min 15 sec

CHANGE SET
Revision 506f17347b06955effb2429216bbd22798e52189 by sitter: (concat at runtime)
  change: edit autotests/testdata.h.cmake
  change: edit autotests/dupetest.cpp


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 
test(s)Failed: TestSuite.dupe

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 82/132 
(62%)CONDITIONAL 35/78 (45%)

By packages
  
default>
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 
14/26 (54%)
autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 40/75 (53%)CONDITIONAL 
21/52 (40%)

Review Request 129028: introduce dupe test from breeze-icons

[Harald Sitter]

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

Review request for KDE Frameworks.


Repository: oxygen-icons5


Description
---

this test uses the external binary fdupes to find duplicated files and
complains about them as they should be symlinked to conserve space


Diffs
-

  CMakeLists.txt 97b8517e88fcda0ff7dd709e3b39fe5476e8efc8 
  autotests/CMakeLists.txt 5fd6b125a2832071c58471b1a27964051252d07c 
  autotests/dupetest.cpp PRE-CREATION 
  autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
  autotests/testhelpers.h PRE-CREATION 

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


Testing
---


Thanks,

Harald Sitter

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Aleix Pol Gonzalez]

> On Sept. 26, 2016, 4:25 p.m., Aleix Pol Gonzalez wrote:
> > Again, I'll refer to how it's done in KCoreAddons. I don't think it's 
> > really useful to need to define variables for every framework you are 
> > compiling.
> 
> Luigi Toscano wrote:
> Is this last sentence part of another conversation that I missed?
> 
> Ralf Habacker wrote:
> This patch does it the same way as already implemented in kcoreaddons 
> https://github.com/KDE/kcoreaddons/blob/master/KF5CoreAddonsConfig.cmake.in#L11.

The part I was referring to is actually:
```
if(CMAKE_CROSSCOMPILING AND KF5_HOST_TOOLING)
find_file(TARGETSFILE KF5CoreAddons/KF5CoreAddonsToolingTargets.cmake PATHS 
${KF5_HOST_TOOLING} ${CMAKE_CURRENT_LIST_DIR} NO_DEFAULT_PATH)
include("${TARGETSFILE}")
else()
```

I don't think we should take into account CROSSCOMPILING if KF5_HOST_TOOLING 
isn't specified. Including the target files should define all the needed 
targets.


- Aleix


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


On Sept. 26, 2016, 4:23 p.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 p.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Ralf Habacker]

> On Sept. 26, 2016, 4:25 nachm., Aleix Pol Gonzalez wrote:
> > Again, I'll refer to how it's done in KCoreAddons. I don't think it's 
> > really useful to need to define variables for every framework you are 
> > compiling.
> 
> Luigi Toscano wrote:
> Is this last sentence part of another conversation that I missed?

This patch does it the same way as already implemented in kcoreaddons 
https://github.com/KDE/kcoreaddons/blob/master/KF5CoreAddonsConfig.cmake.in#L11.


- Ralf


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


On Sept. 26, 2016, 4:23 nachm., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 nachm.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Jenkins-kde-ci: breeze-icons master kf5-qt5 » Linux,gcc - Build # 298 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/298/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 26 Sep 2016 14:30:40 +
Build duration: 3 min 46 sec

CHANGE SET
Revision d23564442dfe782bac9a003e6abacc0966d89f47 by sitter: (fail directly)
  change: edit autotests/dupetest.cpp


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 
test(s)Failed: TestSuite.dupe

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 82/132 
(62%)CONDITIONAL 35/78 (45%)

By packages
  
default>
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 
14/26 (54%)
autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 40/75 (53%)CONDITIONAL 
21/52 (40%)

Jenkins-kde-ci: breeze-icons master stable-kf5-qt5 » Linux,gcc - Build # 296 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/breeze-icons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/296/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 26 Sep 2016 14:30:40 +
Build duration: 3 min 34 sec

CHANGE SET
Revision d23564442dfe782bac9a003e6abacc0966d89f47 by sitter: (fail directly)
  change: edit autotests/dupetest.cpp


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 
test(s)Failed: TestSuite.dupe

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 82/132 
(62%)CONDITIONAL 35/78 (45%)

By packages
  
default>
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 42/57 (74%)CONDITIONAL 
14/26 (54%)
autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 40/75 (53%)CONDITIONAL 
21/52 (40%)

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Luigi Toscano]

> On Sept. 26, 2016, 4:25 p.m., Aleix Pol Gonzalez wrote:
> > Again, I'll refer to how it's done in KCoreAddons. I don't think it's 
> > really useful to need to define variables for every framework you are 
> > compiling.

Is this last sentence part of another conversation that I missed?


- Luigi


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


On Sept. 26, 2016, 4:23 p.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 p.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Aleix Pol Gonzalez]

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



Again, I'll refer to how it's done in KCoreAddons. I don't think it's really 
useful to need to define variables for every framework you are compiling.

- Aleix Pol Gonzalez


On Sept. 26, 2016, 4:23 p.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129025/
> ---
> 
> (Updated Sept. 26, 2016, 4:23 p.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Enable cross compilation support for packages depending on normally installed 
> kdoctools tools.
> 
> 
> Diffs
> -
> 
>   KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
> 
> Diff: https://git.reviewboard.kde.org/r/129025/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 129024: fail the entire dupe test if fdupes is not present

[Harald Sitter]

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

(Updated Sept. 26, 2016, 2:24 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 92fda2493cc40fe8a243722f5a6630ae050d4367 by Harald Sitter 
to branch master.


Repository: breeze-icons


Description
---

ctest doesn't know about skip or anything but fail, so in the interest of
getting reports on this fail the test


Diffs
-

  autotests/dupetest.cpp bf1a67ccff789b5d3bdbe1c730d8c3441691a025 

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


Testing
---


Thanks,

Harald Sitter

Review Request 129025: Enable cross compilation support for packages depending on normally installed kdoctools tools.

[Ralf Habacker]

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

Review request for Documentation, KDE Frameworks and Luigi Toscano.


Repository: kdoctools


Description
---

Enable cross compilation support for packages depending on normally installed 
kdoctools tools.


Diffs
-

  KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 

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


Testing
---


Thanks,

Ralf Habacker

Re: Review Request 129024: fail the entire dupe test if fdupes is not present

[Aleix Pol Gonzalez]

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


Ship it!




Ship It!

- Aleix Pol Gonzalez


On Sept. 26, 2016, 4:21 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129024/
> ---
> 
> (Updated Sept. 26, 2016, 4:21 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> ctest doesn't know about skip or anything but fail, so in the interest of
> getting reports on this fail the test
> 
> 
> Diffs
> -
> 
>   autotests/dupetest.cpp bf1a67ccff789b5d3bdbe1c730d8c3441691a025 
> 
> Diff: https://git.reviewboard.kde.org/r/129024/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

Review Request 129024: fail the entire dupe test if fdupes is not present

[Harald Sitter]

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

Review request for KDE Frameworks.


Repository: breeze-icons


Description
---

ctest doesn't know about skip or anything but fail, so in the interest of
getting reports on this fail the test


Diffs
-

  autotests/dupetest.cpp bf1a67ccff789b5d3bdbe1c730d8c3441691a025 

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


Testing
---


Thanks,

Harald Sitter

Re: Review Request 128969: Add cross compiling support for docbookl10nhelper.

[Ralf Habacker]

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

(Updated Sept. 26, 2016, 4:16 nachm.)


Review request for Documentation, KDE Frameworks and Luigi Toscano.


Repository: kdoctools


Description
---

Add cross compiling support for docbookl10nhelper.


Diffs (updated)
-

  CMakeLists.txt 3394d4a6091ae79ab1d81345318ba879cc15b909 
  KF5DocToolsConfig.cmake.in 9224fd2788aee5db4340cd0ac8115c1a06ca8ebe 
  src/CMakeLists.txt 48b61e95b0ec9f0b19dd312cc7bd805f9dd58fd2 

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


Testing
---

compiled at 
https://build.opensuse.org/package/show/home:rhabacker:branches:KDE:Frameworks526/kdoctools


Thanks,

Ralf Habacker

Re: Review Request 128969: Add cross compiling support for docbookl10nhelper.

[Luigi Toscano]

> On Sept. 21, 2016, 1:52 p.m., Luigi Toscano wrote:
> > src/CMakeLists.txt, line 173
> > 
> >
> > docbookl10nhelper is not meant to be installed
> 
> Ralf Habacker wrote:
> To cross compile kdoctools for windows on e.g. linux native running tools 
> like meinproc5, docbookl10nhelper and checkXML5 are required. On 
> bootstrapping cross compile these native tools are created and packaged 
> together in a packages named mingwXX-cross-kf5-tools like similar cross 
> helper tools for example mingw32-cross-gcc.
> 
> Luigi Toscano wrote:
> Uhm, but normally we don't need it - can't this installation be done 
> under some condition, so that the default is not changed?
> 
> Ralf Habacker wrote:
> Sonnet for example installs similar tools 'parsetrigrams' and  
> 'gentrigrams' also unconditional. On opensuse they are packaged as part of 
> the development package. see 
> https://build.opensuse.org/package/view_file/KDE:Frameworks5/sonnet/sonnet.spec?expand=1
>  line 134,135.
> 
> If this is an absolute nogo there would be still the choice to use a 
> specific configure time switch like INSTALL_ALL or similar ?
> 
> Luigi Toscano wrote:
> docbookl10nhelper only exists because I was unable to do the same tasks 
> with cmake (or maybe it's just not possible). So it's really part of the 
> compilation system.
> 
> My comment is not an "absolute no go" to this change, it's a "I'm fine if 
> it does not change the default behavior". For example mainproc5 can be 
> compiled without bz2 support *only* for internal usage on our server for 
> documentation, but it's not the default and it's clearly marked as such.
> 
> Ralf Habacker wrote:
> kdoctools is configured with MEINPROC_NO_KARCHIVE supporting this special 
> case, which like very meinproc specific. Because there are at least three kf5 
> related packages (kdoctools, ktexteditor and sonnet) which requires to 
> install internal tools I suggest to add 
> 
> option(INSTALL_INTERNAL_TOOLS ...) 
> 
> to the mentioned packages and to install internals tools only if set. I 
> would update this and create related review requests.

Yes, thank you. I was not suggesting to use MEINPROC_NO_ARCHIVE, it was only an 
example of special behavior tailored for a special case, which does not affect 
the default.


- Luigi


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


On Sept. 21, 2016, 11:43 a.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128969/
> ---
> 
> (Updated Sept. 21, 2016, 11:43 a.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Add cross compiling support for docbookl10nhelper.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6f903b542bc9ba256dd379275567d5ff2127fe39 
> 
> Diff: https://git.reviewboard.kde.org/r/128969/diff/
> 
> 
> Testing
> ---
> 
> compiled at 
> https://build.opensuse.org/package/show/home:rhabacker:branches:KDE:Frameworks526/kdoctools
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 128969: Add cross compiling support for docbookl10nhelper.

[Ralf Habacker]

> On Sept. 21, 2016, 1:52 nachm., Luigi Toscano wrote:
> > src/CMakeLists.txt, line 173
> > 
> >
> > docbookl10nhelper is not meant to be installed
> 
> Ralf Habacker wrote:
> To cross compile kdoctools for windows on e.g. linux native running tools 
> like meinproc5, docbookl10nhelper and checkXML5 are required. On 
> bootstrapping cross compile these native tools are created and packaged 
> together in a packages named mingwXX-cross-kf5-tools like similar cross 
> helper tools for example mingw32-cross-gcc.
> 
> Luigi Toscano wrote:
> Uhm, but normally we don't need it - can't this installation be done 
> under some condition, so that the default is not changed?
> 
> Ralf Habacker wrote:
> Sonnet for example installs similar tools 'parsetrigrams' and  
> 'gentrigrams' also unconditional. On opensuse they are packaged as part of 
> the development package. see 
> https://build.opensuse.org/package/view_file/KDE:Frameworks5/sonnet/sonnet.spec?expand=1
>  line 134,135.
> 
> If this is an absolute nogo there would be still the choice to use a 
> specific configure time switch like INSTALL_ALL or similar ?
> 
> Luigi Toscano wrote:
> docbookl10nhelper only exists because I was unable to do the same tasks 
> with cmake (or maybe it's just not possible). So it's really part of the 
> compilation system.
> 
> My comment is not an "absolute no go" to this change, it's a "I'm fine if 
> it does not change the default behavior". For example mainproc5 can be 
> compiled without bz2 support *only* for internal usage on our server for 
> documentation, but it's not the default and it's clearly marked as such.

kdoctools is configured with MEINPROC_NO_KARCHIVE supporting this special case, 
which like very meinproc specific. Because there are at least three kf5 related 
packages (kdoctools, ktexteditor and sonnet) which requires to install internal 
tools I suggest to add 

option(INSTALL_INTERNAL_TOOLS ...) 

to the mentioned packages and to install internals tools only if set. I would 
update this and create related review requests.


- Ralf


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


On Sept. 21, 2016, 11:43 vorm., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128969/
> ---
> 
> (Updated Sept. 21, 2016, 11:43 vorm.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Add cross compiling support for docbookl10nhelper.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6f903b542bc9ba256dd379275567d5ff2127fe39 
> 
> Diff: https://git.reviewboard.kde.org/r/128969/diff/
> 
> 
> Testing
> ---
> 
> compiled at 
> https://build.opensuse.org/package/show/home:rhabacker:branches:KDE:Frameworks526/kdoctools
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Harald Sitter]

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

(Updated Sept. 26, 2016, 3:56 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 343b7ecce2c1a7e42a7a913258d65e993b603bce by Harald Sitter 
to branch master.


Repository: breeze-icons


Description
---

run a duplicate detection program on the icon dir trees to detect
duplicated icons which should be symlinked to reduce disk usage

uses external program since otherwise we'd be needlessly reimplmenting what 
that program does (alas, couldn't find a lib)


Diffs
-

  autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
  autotests/dupetest.cpp PRE-CREATION 
  autotests/newlinetest.cpp 355af0c3d0e1ce42c260652e43a00364769661f1 
  autotests/symlinktest.cpp 15047ea72f95cbaa4578668e59ad687faa8ae230 
  autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
  autotests/testhelpers.h PRE-CREATION 

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


Testing
---

correctly detects all the nasty dupes


Thanks,

Harald Sitter

Re: Review Request 128970: Add cross compile support for meinproc5.

[Ralf Habacker]

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

(Updated Sept. 26, 2016, 1:45 p.m.)


Status
--

This change has been marked as submitted.


Review request for Documentation, KDE Frameworks and Luigi Toscano.


Changes
---

Submitted with commit d6f7aa2a1641569e119ba81697fe29104b82fb5e by Ralf Habacker 
to branch master.


Repository: kdoctools


Description
---

Add cross compile support for meinproc5.


Diffs
-

  src/CMakeLists.txt 6f903b542bc9ba256dd379275567d5ff2127fe39 

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


Testing
---

compiled at 
https://build.opensuse.org/package/show/home:rhabacker:branches:KDE:Frameworks526/kdoctools


Thanks,

Ralf Habacker

Re: Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Aleix Pol Gonzalez]

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


Ship it!




Let's get this in for now, so we can actually fix stuff?

- Aleix Pol Gonzalez


On Sept. 26, 2016, 2:30 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129021/
> ---
> 
> (Updated Sept. 26, 2016, 2:30 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> run a duplicate detection program on the icon dir trees to detect
> duplicated icons which should be symlinked to reduce disk usage
> 
> uses external program since otherwise we'd be needlessly reimplmenting what 
> that program does (alas, couldn't find a lib)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
>   autotests/dupetest.cpp PRE-CREATION 
>   autotests/newlinetest.cpp 355af0c3d0e1ce42c260652e43a00364769661f1 
>   autotests/symlinktest.cpp 15047ea72f95cbaa4578668e59ad687faa8ae230 
>   autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
>   autotests/testhelpers.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129021/diff/
> 
> 
> Testing
> ---
> 
> correctly detects all the nasty dupes
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

Re: Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Aleix Pol Gonzalez]

> On Sept. 26, 2016, 1:40 p.m., Aleix Pol Gonzalez wrote:
> > autotests/dupetest.cpp, line 66
> > 
> >
> > It should mark an error if dupes found, no?
> 
> Harald Sitter wrote:
> Doesn't QFAIL unwind the stack?

Not if you do it in the right place :D


- Aleix


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


On Sept. 26, 2016, 2:30 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129021/
> ---
> 
> (Updated Sept. 26, 2016, 2:30 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> run a duplicate detection program on the icon dir trees to detect
> duplicated icons which should be symlinked to reduce disk usage
> 
> uses external program since otherwise we'd be needlessly reimplmenting what 
> that program does (alas, couldn't find a lib)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
>   autotests/dupetest.cpp PRE-CREATION 
>   autotests/newlinetest.cpp 355af0c3d0e1ce42c260652e43a00364769661f1 
>   autotests/symlinktest.cpp 15047ea72f95cbaa4578668e59ad687faa8ae230 
>   autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
>   autotests/testhelpers.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129021/diff/
> 
> 
> Testing
> ---
> 
> correctly detects all the nasty dupes
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

Re: Review Request 128831: Check whether kwallet is enabled in Wallet::isOpen(name)

[Elvis Angelaccio]

> On Sept. 24, 2016, 12:40 p.m., Elvis Angelaccio wrote:
> > src/api/KWallet/kwallet.cpp, line 366
> > 
> >
> > You should probably use isEnabled() instead, in case someone is using 
> > ksecretservice (though m_walletEnabled is already used all over the 
> > place)...
> > 
> > Anyway +1
> 
> Wolfgang Bauer wrote:
> This is inside the non-ksecretservice codepath, so it makes no difference 
> whether one uses m_walletEnabled or isEnabled() (not to mention that I think 
> the distinction is not even necessary in isEnabled(), AFAICS m_walletEnabled 
> is set accordingly in the ksecretservice case too).
> 
> And as you write the check is like that everywhere else in the code.
> I don't think it makes sense to deviate here in this one method, if 
> that's desirable it should be changed everywhere in a separate commit IMHO.
> 
> In case you mean that the check should be done using isEnabled() right at 
> the start of the method (i.e. before the if):
> I'm not sure this check is needed at all in the ksecretservice case, 
> there is no ->getInterface().xxx call that could crash.
> 
> KWalletDLauncher does check whether the wallet is enabled and just 
> returns if not without doing anything. This means it doesn't start the 
> KWallet service in the non-ksecretservice, that's what causes the mentioned 
> crash in the first place because a nullptr is dereferenced.
> 
> I don't know anything about ksecretservice, but the code here probably 
> will return false in case the wallet is disabled anyway (at least that's my 
> feeling after looking at this code).
> 
> Elvis Angelaccio wrote:
> > This is inside the non-ksecretservice codepath, so it makes no 
> difference whether one uses m_walletEnabled or isEnabled() (not to mention 
> that I think the distinction is not even necessary in isEnabled(), AFAICS 
> m_walletEnabled is set accordingly in the ksecretservice case too).
> And as you write the check is like that everywhere else in the code.
> I don't think it makes sense to deviate here in this one method, if 
> that's desirable it should be changed everywhere in a separate commit IMHO.
> 
> It probably makes more sense, yes. Ignore my comment then :)

Meanwhile I realized that the ksecretservice code path is broken, so there is 
no way that someone is using it.


- Elvis


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


On Sept. 4, 2016, 9:20 p.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128831/
> ---
> 
> (Updated Sept. 4, 2016, 9:20 p.m.)
> 
> 
> Review request for KDE Frameworks and Valentin Rusu.
> 
> 
> Bugs: 358260
> https://bugs.kde.org/show_bug.cgi?id=358260
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> If kwallet is disabled, walletlauncher() fails and 
> walletLauncher()->getInterface().isOpen(name) causes a crash.
> This affects e.g. drkonqi, but probably also other applications.
> 
> Return false in this case, if kwallet is disabled a wallet cannot be open 
> either.
> 
> 
> Diffs
> -
> 
>   src/api/KWallet/kwallet.cpp dffebda 
> 
> Diff: https://git.reviewboard.kde.org/r/128831/diff/
> 
> 
> Testing
> ---
> 
> - disable kwallet in the settings
> - start a KF5 application (say, dolphin)
> - make it crash, e.g. via "killall -ILL dolphin"
> - try to report the crash with drkonqi
> 
> When you get to the page where you have to enter the bugzilla 
> username/password, drkonqi crashed, with this patch it doesn't crash.
> 
> drkonqi is also still able to get the username/password from kwallet if it is 
> enabled.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

Re: Review Request 128970: Add cross compile support for meinproc5.

[Luigi Toscano]

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


Ship it!




Ship It!

- Luigi Toscano


On Sept. 21, 2016, 11:44 a.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128970/
> ---
> 
> (Updated Sept. 21, 2016, 11:44 a.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Add cross compile support for meinproc5.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6f903b542bc9ba256dd379275567d5ff2127fe39 
> 
> Diff: https://git.reviewboard.kde.org/r/128970/diff/
> 
> 
> Testing
> ---
> 
> compiled at 
> https://build.opensuse.org/package/show/home:rhabacker:branches:KDE:Frameworks526/kdoctools
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 128970: Add cross compile support for meinproc5.

[Luigi Toscano]

> On Sept. 21, 2016, 1:52 p.m., Luigi Toscano wrote:
> > src/CMakeLists.txt, line 60
> > 
> >
> > So, I inherited kdoctools with this code commented - do you have an 
> > idea why it was like this, and why your patch would solve the (old) issues, 
> > or if they are not relevant anymore? My cross-compilation knowledge is 
> > limited.
> 
> Ralf Habacker wrote:
> The mentioned message came from the commit 
> https://github.com/KDE/kdelibs/commit/39dfbe0910228f0dccc0297e59ba47cf7e9b0d7c
>  and indicates that the initial attempt has been reverted because of 
> unfinished state.
> 
> The attempt this commit belongs to has been finished and is working (see 
> https://build.opensuse.org/project/show/home:rhabacker:branches:windows:mingw:win32:KF526)
>  and specific for kdoctools here 
> https://build.opensuse.org/package/show/home:rhabacker:branches:windows:mingw:win32:KF526/mingw32-kdoctools
> 
> If you want I can refactor the patch to let this commented out stuff 
> inside.

I see; the codepath for the non-crosscompiling case is untouched, so I'm fine 
then.


- Luigi


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


On Sept. 21, 2016, 11:44 a.m., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128970/
> ---
> 
> (Updated Sept. 21, 2016, 11:44 a.m.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Add cross compile support for meinproc5.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6f903b542bc9ba256dd379275567d5ff2127fe39 
> 
> Diff: https://git.reviewboard.kde.org/r/128970/diff/
> 
> 
> Testing
> ---
> 
> compiled at 
> https://build.opensuse.org/package/show/home:rhabacker:branches:KDE:Frameworks526/kdoctools
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 128970: Add cross compile support for meinproc5.

[Ralf Habacker]

> On Sept. 21, 2016, 1:52 nachm., Luigi Toscano wrote:
> > src/CMakeLists.txt, line 60
> > 
> >
> > So, I inherited kdoctools with this code commented - do you have an 
> > idea why it was like this, and why your patch would solve the (old) issues, 
> > or if they are not relevant anymore? My cross-compilation knowledge is 
> > limited.

The mentioned message came from the commit 
https://github.com/KDE/kdelibs/commit/39dfbe0910228f0dccc0297e59ba47cf7e9b0d7c 
and indicates that the initial attempt has been reverted because of unfinished 
state.

The attempt this commit belongs to has been finished and is working (see 
https://build.opensuse.org/project/show/home:rhabacker:branches:windows:mingw:win32:KF526)
 and specific for kdoctools here 
https://build.opensuse.org/package/show/home:rhabacker:branches:windows:mingw:win32:KF526/mingw32-kdoctools

If you want I can refactor the patch to let this commented out stuff inside.


- Ralf


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


On Sept. 21, 2016, 11:44 vorm., Ralf Habacker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128970/
> ---
> 
> (Updated Sept. 21, 2016, 11:44 vorm.)
> 
> 
> Review request for Documentation, KDE Frameworks and Luigi Toscano.
> 
> 
> Repository: kdoctools
> 
> 
> Description
> ---
> 
> Add cross compile support for meinproc5.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6f903b542bc9ba256dd379275567d5ff2127fe39 
> 
> Diff: https://git.reviewboard.kde.org/r/128970/diff/
> 
> 
> Testing
> ---
> 
> compiled at 
> https://build.opensuse.org/package/show/home:rhabacker:branches:KDE:Frameworks526/kdoctools
> 
> 
> Thanks,
> 
> Ralf Habacker
> 
>

Re: Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Harald Sitter]

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

(Updated Sept. 26, 2016, 12:30 p.m.)


Review request for KDE Frameworks.


Changes
---

drop q_foreach


Repository: breeze-icons


Description
---

run a duplicate detection program on the icon dir trees to detect
duplicated icons which should be symlinked to reduce disk usage

uses external program since otherwise we'd be needlessly reimplmenting what 
that program does (alas, couldn't find a lib)


Diffs (updated)
-

  autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
  autotests/dupetest.cpp PRE-CREATION 
  autotests/newlinetest.cpp 355af0c3d0e1ce42c260652e43a00364769661f1 
  autotests/symlinktest.cpp 15047ea72f95cbaa4578668e59ad687faa8ae230 
  autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
  autotests/testhelpers.h PRE-CREATION 

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


Testing
---

correctly detects all the nasty dupes


Thanks,

Harald Sitter

Re: Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Harald Sitter]

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

(Updated Sept. 26, 2016, 12:23 p.m.)


Review request for KDE Frameworks.


Changes
---

refactor code copy away


Repository: breeze-icons


Description
---

run a duplicate detection program on the icon dir trees to detect
duplicated icons which should be symlinked to reduce disk usage

uses external program since otherwise we'd be needlessly reimplmenting what 
that program does (alas, couldn't find a lib)


Diffs (updated)
-

  autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
  autotests/dupetest.cpp PRE-CREATION 
  autotests/newlinetest.cpp 355af0c3d0e1ce42c260652e43a00364769661f1 
  autotests/symlinktest.cpp 15047ea72f95cbaa4578668e59ad687faa8ae230 
  autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
  autotests/testhelpers.h PRE-CREATION 

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


Testing
---

correctly detects all the nasty dupes


Thanks,

Harald Sitter

Re: Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Harald Sitter]

> On Sept. 26, 2016, 11:40 a.m., Aleix Pol Gonzalez wrote:
> > autotests/dupetest.cpp, line 66
> > 
> >
> > It should mark an error if dupes found, no?

Doesn't QFAIL unwind the stack?


- Harald


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


On Sept. 26, 2016, 11:27 a.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129021/
> ---
> 
> (Updated Sept. 26, 2016, 11:27 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> run a duplicate detection program on the icon dir trees to detect
> duplicated icons which should be symlinked to reduce disk usage
> 
> uses external program since otherwise we'd be needlessly reimplmenting what 
> that program does (alas, couldn't find a lib)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
>   autotests/dupetest.cpp PRE-CREATION 
>   autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
> 
> Diff: https://git.reviewboard.kde.org/r/129021/diff/
> 
> 
> Testing
> ---
> 
> correctly detects all the nasty dupes
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

Re: Review Request 129019: KFileItemActions: add addPluginActionsTo(QMenu *).

[Anthony Fieroni]

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




src/widgets/kfileitemactions.cpp (line 487)


factory can be nullptr


- Anthony Fieroni


On Sept. 26, 2016, 11:10 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129019/
> ---
> 
> (Updated Sept. 26, 2016, 11:10 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This was missing in KFileItemActions so apps had to implement
> this by themselves (this particular implementation is inspired by Dolphin's,
> while KonqPopupMenu only had support for .desktop files and not JSON 
> metadata).
> 
> This API will also be useful in folderview (which I'm porting away
> from KonqPopupMenu) and kfind (same strategy).
> 
> 
> Diffs
> -
> 
>   src/widgets/kfileitemactions.h 9b025549efdceef7f32dc461f3d5bfa2ee10d938 
>   src/widgets/kfileitemactions.cpp b31da0a38bfef634b0be207be9a87c9ba6864230 
>   src/widgets/kfileitemactions_p.h 3f669feb3cf54b61dcb7ad0a22bc005210e18899 
> 
> Diff: https://git.reviewboard.kde.org/r/129019/diff/
> 
> 
> Testing
> ---
> 
> Porting dolphin, folderview and kfind to this.
> 
> Still debugging why some service menu entries appear duplicated though - 
> might or might not be related to this.
> 
> 
> Thanks,
> 
> David Faure
> 
>

Re: Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Aleix Pol Gonzalez]

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




autotests/dupetest.cpp (line 36)


`const` missing



autotests/dupetest.cpp (line 66)


It should mark an error if dupes found, no?


- Aleix Pol Gonzalez


On Sept. 26, 2016, 1:27 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129021/
> ---
> 
> (Updated Sept. 26, 2016, 1:27 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> run a duplicate detection program on the icon dir trees to detect
> duplicated icons which should be symlinked to reduce disk usage
> 
> uses external program since otherwise we'd be needlessly reimplmenting what 
> that program does (alas, couldn't find a lib)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
>   autotests/dupetest.cpp PRE-CREATION 
>   autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 
> 
> Diff: https://git.reviewboard.kde.org/r/129021/diff/
> 
> 
> Testing
> ---
> 
> correctly detects all the nasty dupes
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

Review Request 129021: add a test to find duplicated files (inspired by similar test in debian)

[Harald Sitter]

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

Review request for KDE Frameworks.


Repository: breeze-icons


Description
---

run a duplicate detection program on the icon dir trees to detect
duplicated icons which should be symlinked to reduce disk usage

uses external program since otherwise we'd be needlessly reimplmenting what 
that program does (alas, couldn't find a lib)


Diffs
-

  autotests/CMakeLists.txt 9e3f841d951afb007cba3da79c217e9c2a3e38da 
  autotests/dupetest.cpp PRE-CREATION 
  autotests/testdata.h.cmake 476cf9ebf8b45c65e5479e14adcb1ea352e1f24d 

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


Testing
---

correctly detects all the nasty dupes


Thanks,

Harald Sitter

Re: Review Request 129020: Fix find invocation in validate_svg.sh on FreeBSD.

[Gleb Popov]

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

(Updated Sept. 26, 2016, 11:49 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 9e0466a1996717e44ca8c3842abd304d69af7ea7 by Gleb Popov to 
branch master.


Repository: breeze-icons


Description
---

Without this patch the build was failing with

```
find: illegal option -- n
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
   find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]
```


Diffs
-

  validate_svg.sh bbc3076 

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


Testing
---


Thanks,

Gleb Popov

Re: Review Request 129020: Fix find invocation in validate_svg.sh on FreeBSD.

[Kevin Funk]

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


Ship it!




Ship It!

- Kevin Funk


On Sept. 26, 2016, 9:45 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129020/
> ---
> 
> (Updated Sept. 26, 2016, 9:45 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Without this patch the build was failing with
> 
> ```
> find: illegal option -- n
> usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
>find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]
> ```
> 
> 
> Diffs
> -
> 
>   validate_svg.sh bbc3076 
> 
> Diff: https://git.reviewboard.kde.org/r/129020/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

Review Request 129020: Fix find invocation in validate_svg.sh on FreeBSD.

[Gleb Popov]

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

Review request for KDE Frameworks.


Repository: breeze-icons


Description
---

Without this patch the build was failing with

```
find: illegal option -- n
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
   find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]
```


Diffs
-

  validate_svg.sh bbc3076 

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


Testing
---


Thanks,

Gleb Popov

Review Request 129019: KFileItemActions: add addPluginActionsTo(QMenu *).

[David Faure]

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

Review request for KDE Frameworks.


Repository: kio


Description
---

This was missing in KFileItemActions so apps had to implement
this by themselves (this particular implementation is inspired by Dolphin's,
while KonqPopupMenu only had support for .desktop files and not JSON metadata).

This API will also be useful in folderview (which I'm porting away
from KonqPopupMenu) and kfind (same strategy).


Diffs
-

  src/widgets/kfileitemactions.h 9b025549efdceef7f32dc461f3d5bfa2ee10d938 
  src/widgets/kfileitemactions.cpp b31da0a38bfef634b0be207be9a87c9ba6864230 
  src/widgets/kfileitemactions_p.h 3f669feb3cf54b61dcb7ad0a22bc005210e18899 

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


Testing
---

Porting dolphin, folderview and kfind to this.

Still debugging why some service menu entries appear duplicated though - might 
or might not be related to this.


Thanks,

David Faure