inclusion of MathematiK into kde-edu

[Alexander Rieder]

On Tuesday 22 September 2009 02:21:38 Michael Pyne wrote:
> 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?) :)
Not there is nothing. But I should probably start putting something together 
soon. I just don't know where/how?
> 
> 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 really know. I've taken the FindR.cmake module from the RKWard 
project, as my CMake skills are quite limited.

> 
> * 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.
removed that one.

> * 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).
Oh, great. I'll change that.

> * The mathematik.kcfgc file appears to be redundant given that you have a
> (properly-spelled) settings.kcfgc file.
Oops... don't know where this one comes from. removed.
> 
> * On that note I don't think it's necessary to run this twice:
> kde4_add_kcfg_files(mathematik_SRCS settings.kcfgc)
thanks for noticing. problably a relict of copy-paste. removed


thanks for your suggestions,

Alexander Rieder

> * 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
>