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