----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107752/#review23767 -----------------------------------------------------------
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. - Laszlo Papp 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 > >