Keysmith in kdereview

[Johan Ouwerkerk]

First of all sorry for the duplicate reply Friedrich. I messed up with
the send button earlier.

Here goes the second try:

>
> Did some usual-nitpick-CMake-code cleanup commit already (things which also
> apply to other new Plasma repos, someone might want to brush over their
> CMakeLists.txt as well, using that commit as reference).
>

Thanks for that one!

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

> * 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)

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`?

> * you might want to check if KF 5.37 has minimum version of Kirigami features
> used, otherwise needs bumping
>

Good point: I've created a MR for this:
https://invent.kde.org/kde/keysmith/merge_requests/26

Regards,

 - Johan