Review Request: Fix icon generation and installation on OS X

Yue Liu yue.liu at mail.com
Thu Dec 20 21:27:06 GMT 2012



> On Dec. 20, 2012, 6:58 p.m., Laszlo Papp wrote:
> > Great, thank you for your care! One suggestion to this, and I think then it is fine from that point of view: you could write a foreach on top of the "copy_icons" macro, and avoid the same function name in each line.
> 
> Yue Liu wrote:
>     that doesn't save many characters but increased complexity since you have to emulate a 2d array.
> 
> Laszlo Papp wrote:
>     You follow the 2D logic either way, but now it is done manually by copy/paste as many times as you need it which is currently ten times the same call.
> 
> Laszlo Papp wrote:
>     Also, the raw data is not separated from the code this way as much as it could be.

sorry i don't get it what's the difference between call that ten times directly and use a foreach loop to call that ten times. And those patterns has to be in the code some where, what's the difference between put them in an array and put them in several adjacent lines.


- Yue


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107752/#review23767
-----------------------------------------------------------


On Dec. 20, 2012, 6:10 p.m., Yue Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107752/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2012, 6:10 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> There are two issues when using kde4_add_app_icon on mac. a) apps using kdeinit won't install icon files to thier app bundles, b) mac app icon generating method is outdated and does not support retina resolution.
> 
> The patch changed kde4_add_kdeinit_executable and kde4_add_app_icon to solve these issues.
> 
> 
> Diffs
> -----
> 
>   cmake/modules/KDE4Macros.cmake 0753879 
> 
> Diff: http://git.reviewboard.kde.org/r/107752/diff/
> 
> 
> Testing
> -------
> 
> Works well on 4.9 branch.
> Not sure if some changes breaks other platforms.
> 
> 
> Thanks,
> 
> Yue Liu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121220/e3b06868/attachment.htm>


More information about the kde-core-devel mailing list