Minuet (music education software) moved to kdereview

[Albert Astals Cid]

El Sunday 24 January 2016, a les 16:50:18, Andreas Cord-Landwehr va escriure:
> Hi Sandro, it is always great when such a cool application lands in KDE Edu.
> I just made a first and rough (since I do not have all dependencies yet to
> really compile and test it) code review.
> 
> Here a some minors I noticed:
> * the application does not link against KCrash (which is needed for
> DrKonqi); also in the main.cpp there should be a call to
> "KCrash::initialize();" * an appdata file is missing
> * in the main CMakeLists.txt, for KF5 there is no minimum version and for
> ECM the minimum version should probably be higher than 1.0.0
> * it looks strange to me that in minuet/cmake/ there are Config-files for
> the 3rd-party library drumstick. My understanding was that such Config
> files should only be shipped with the respective library (maybe someone
> with a deeper CMake-knowledge can comment?)
> * qml-file-localization: It looks strange to me that you are mixing two
> different localization systems with KI18n (in all C++ and UI files) and qsTr
> in all QML files. Since I am not familiar with qsTr, I cannot really
> comment on how it works and if everything is setup correctly; at least
> Messages.sh do not process qml files and hence do not extract strings from
> them, but that might only be necessary for Ki18n?

Yeah, you should use ki18n in the qml files too.

Cheers,
  Albert

> 
> Other than that, the code quality and licensing information look really
> good.
> 
> Cheers,
> Andreas