Minuet (music education software) moved to kdereview

Andreas Cord-Landwehr cordlandwehr at kde.org
Sun Jan 24 15:50:18 GMT 2016


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?

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

Cheers,
Andreas




More information about the kde-core-devel mailing list