inclusion of MathematiK into kde-edu

[Michael Pyne]

On Monday 21 September 2009 16:19:45 Alexander Rieder wrote:
> I'm looking forward to get your reviews,

You'll have to please excuse me because I am very lazy by this point but is 
there a website (or more to the point, screenshots?) :)

Also from a brief glance:

* The FindLibSpectre.cmake is from kdegraphics.  This isn't a question for the 
author but I wonder what our policy is for merging FindFoo modules so that we 
avoid code duplication?

* The FindR.cmake seems overly wordy.  Also shouldn't there be a lapack pkg-
config module in the case that Rlapack is present?  That should give you the 
list of libraries (such as gfortran) to link in addition to -lRlapack without 
guessing.  It's present on Gentoo at least, not sure of other distros.

* I don't think it's necessary to add_directory(cmake) from the root 
CMakeLists.txt since you're already added the module directory to use.

* You use a HelpExtension proxy of some sort to forward showHelp() signals out 
of the part you're using to the main app.  You should just be able to add that 
signal to your part itself though without having to use a proxy object, since 
the signal is looked up at run time.  (For this reason even if you go the 
HelpExtension route you don't need the helpextension.h header included in your 
mathematik.cpp for your MathematiKShell::addWorksheet(), the connect call is
against QObject).

* The mathematik.kcfgc file appears to be redundant given that you have a 
(properly-spelled) settings.kcfgc file.

* On that note I don't think it's necessary to run this twice: 
kde4_add_kcfg_files(mathematik_SRCS settings.kcfgc)

* The code itself seems straightforward enough from a very cursory review and 
appears to have met Krazy at least once, which is good.

Regards,
 - Michael Pyne
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090921/cbf8c4de/attachment.sig>