Keysmith in kdereview

[Friedrich W. H. Kossebau]

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
> 
> <kossebau at 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