On Monday 08 of July 2013 19:19:08 David Edmundson wrote: > Code wise, things looks pretty good. > > Minor comments: > - the library is GPL, not LGPL which is the norm for libraries. > Is this deliberate?
No, I don't think so. I'm just fine with changing it to LGPL (and I guess Alex will be too). > > - Inside kscreen you have a copy of the metadata.desktop file twice. > Just have the one and install it to the two places. Fixed. > You might also want to use the version number from your main CMakeLists.txt > in your .desktop file too, otherwise updating can get messy. Fixed, thanks. > > -Generator::biggestOutput > rename the local variable "total"...it's not a total of anything, I had to > re-read it 10 times to realise the code was correct. Right, confused me too :-) Fixed. > In general though +1 from me. Thanks :) Dan -- Daniel Vrátil KDE Desktop Team Associate Software Engineer, Red Hat, Inc. GPG Key: 0xC59D614F6F4AE348 Fingerprint: 4EC1 86E3 C54E 0B39 5FDD B5FB C59D 614F 6F4A E348
signature.asc
Description: This is a digitally signed message part.
