> On Jan. 11, 2016, 5: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?

>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.


- Gleb


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


On Jan. 10, 2016, 11 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, 11 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
> 
>

_______________________________________________
Kde-buildsystem mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Reply via email to