[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Moving KScreen and libkscreen to extragear
From:       Daniel =?ISO-8859-1?Q?Vr=E1til?= <dvratil () redhat ! com>
Date:       2013-10-18 8:48:53
Message-ID: 16261326.9Yd8MYeq5Z () odin
[Download RAW message or body]


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" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]