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

List:       kde-core-devel
Subject:    Re: Review request: Kcmgrub2
From:       Raphael Kubo da Costa <kubito () gmail ! com>
Date:       2011-04-05 22:28:21
Message-ID: 87hbacz62i.fsf () gmail ! com
[Download RAW message or body]

Alberto Mattea <alberto@mattea.info> writes:

> Hi all,
> after 4 releases I think kcmgrub2 has reached an acceptable level of maturity,
> so I'd ask for a move to kdereview. It is currently in playground-sysadmin
> (git).

I only took a quick look, as my Py{Qt,KDE}-fu is not that good.

Buildsystem-wise:

  * I did not understand why you used include() instead of
find_package() in, for example,

      include(FindPyQt4)

  * It's probably a good idea to add some kind of README for packagers
explaining what the dependencies are.

Licensing-wise:

  * Don't you need to add the appropriate license header to your code
files?

As for kcmgrub2.py itself:

  * It might be better to set the WhatsThis values in the .ui file
itself.
  * `x' is not a good name for a global variable.
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic