Sending my reply on-list while Johan's resent reply to list seems stuck in moderation:
Am Samstag, 28. Dezember 2019, 15:03:33 CET schrieb Johan Ouwerkerk: > On Wed, Dec 18, 2019 at 5:43 PM Friedrich W. H. Kossebau > > <kosse...@kde.org> wrote: > > Other things noticed on superficial look: > > * UI not translated, i18n support setup missing completely > > Yes, there is an issue for that on invent: > https://invent.kde.org/kde/keysmith/issues/5 That one is a blocker though to pass kdereview, for what I understand from https://community.kde.org/ReleasingSoftware#Sanity_Checklist as linked from https://community.kde.org/Policies/Application_Lifecycle#kdereview At least personally I would expect this to be a minimum requirement for software created officially in the KDE community. Perhaps new generation of KDE develpoers thinks differently, but in that case please reconsider whether i18n is not a fundamental need :) > > * uses own "ENABLE_TESTING", not "BUILD_TESTING" flag from > > KDECompilerSettings> > > proposed: > > + switch flag use to BUILD_TESTING > > - remove option(ENABLE_TESTING "Enable tests" ON) > > - remove enable_testing() (done by KDECompilerSettings) My bad, s/KDECompilerSettings/KDECMakeSettings/g here. > I'm not entirely sure what the origins of this are but see also the CI > template for building flatpaks: > https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/binary-flatpak. > yml As you can see from the example usage the `-DENABLE_TESTING` flag is > suggested there. > > Speaking as a developer I don't really care what it is called, but I > would like it to be on by default. Is that the case for > `BUILD_TESTING`? Main motivation here is consistency in the code created in the KDE community, so people working across KDE projects do not have to switch mind all the time. Yes, BUILD_TESTING is ON by default, either indirectly via CTest or explicit, see https://phabricator.kde.org/source/extra-cmake-modules/browse/master/kde-modules/KDECMakeSettings.cmake$190 and https://cmake.org/cmake/help/latest/ module/CTest.html The people doing flatpaks want to fix the template, please poke them to do so (did already a comment on the commit myself, but someone needs to run after this to also happen :) ). Cheers Friedrich
