Review Request 126711: ECMAddAppIcon: Use absolute path when operating on icons
Alex Merry
alex.merry at kde.org
Tue Jan 12 19:56:52 UTC 2016
> On Jan. 11, 2016, 2:53 p.m., Alex Merry wrote:
> > The commands that deal with the icons are run from CMAKE_CURRENT_SOURCE_DIR with the intention of making relative paths work. I take it this isn't sufficient?
> >
> > It would be nice to have tests for this module, including ones covering this case. I realise it would be hard to check that icons are embedded properly, but it would be able to check that it didn't fail, and that it generated the files that it should, and updated the variable that was passed to it. Is this something you would be willing to look at?
>
> Gleb Popov wrote:
> >The commands that deal with the icons are run from CMAKE_CURRENT_SOURCE_DIR with the intention of making relative paths work. I take it this isn't sufficient?
>
> Yes, the `if(EXISTS $[icon}` check was failing for relative path. This is why Kate, for instance, prepends ${CMAKE_CURRENT_SOURCE_DIR} for its icons. See https://quickgit.kde.org/?p=kate.git&a=blob&h=3b24e088073f7f8893c0bec4d58894957982f28d&hb=371e7a2fda5fe5e98173020cd8553b2181e125c9&f=kate%2FCMakeLists.txt#l75
>
> >It would be nice to have tests for this module
> >Is this something you would be willing to look at?
>
> Not willing, to be honest, but can do this if you want.
I'm not going to make you do it to get a shipit from me, given that no tests have been written for this module already, but I thought it was worth a try :-).
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126711/#review90901
-----------------------------------------------------------
On Jan. 10, 2016, 8 p.m., Gleb Popov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126711/
> -----------------------------------------------------------
>
> (Updated Jan. 10, 2016, 8 p.m.)
>
>
> Review request for Extra Cmake Modules and Kevin Funk.
>
>
> Repository: extra-cmake-modules
>
>
> Description
> -------
>
> This makes it optional to list icons with ${CMAKE_CURRENT_SOURCE_DIR}/ prefix.
>
>
> Diffs
> -----
>
> modules/ECMAddAppIcon.cmake 5233a5f
>
> Diff: https://git.reviewboard.kde.org/r/126711/diff/
>
>
> Testing
> -------
>
> Built KDevelop on Windows, the icon is here.
>
>
> Thanks,
>
> Gleb Popov
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20160112/3ddcd6d3/attachment.html>
More information about the Kde-buildsystem
mailing list