Hi, thanks for the review. In data mercoledì 6 aprile 2011 00:28:21, Raphael Kubo da Costa ha scritto: > Alberto Mattea <albe...@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)
Actually I used an article on the KDE wiki as an example, I didn't know it wasn't the standard way of doing it. Now it's fixed. > > * It's probably a good idea to add some kind of README for packagers > explaining what the dependencies are. Ok, done. > > Licensing-wise: > > * Don't you need to add the appropriate license header to your code > files? Yep :-). Done. > > As for kcmgrub2.py itself: > > * It might be better to set the WhatsThis values in the .ui file > itself. Well, I thought about this, but ultimately I chose to put them in the code so that in the future I may make them dynamic (example: if python-xlib is not available explain why in the resolution whatsthis). > * `x' is not a good name for a global variable. You're right, I changed it to xlibAvailable. Thanks again, Alberto
signature.asc
Description: This is a digitally signed message part.