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