Haruna in kdereview

[Harald Sitter]

It's an amazing app! Really awesome.

- you might want to consider using a newer version in your
find_package call for ECM. ECM does enable more modern/useful features
but only when used with a suitably high version. so, I'd put this at
the actual minimal version you want to support
- the color-schemes dir seems to do nothing. it installs files that
don't actually exist in the source. fix it or rm -rf?
- for the screenshots.md and the metainfo.xml you should consider
using our screenshot CDN instead
https://invent.kde.org/websites/product-screenshots
- not sure how big of a concern that will be in practice, but you
should be careful with calling mimeTypeForFile, it is potentially
costly and a local path may still be backed by a network'd mount. when
in doubt it's probably better to check KFileItem::isSlow() first and
use mimeTypeForUrl when isSlow is true (or resolve the mimetype in a
qfuture/thread)
- Application::aboutApplication: we do have an AboutPage in klirigami
these days, maybe check that out so you can get rid of the qwidget
dialog
- you should codify the runtime dep on youtube-dl via cmake. make a
dummy finder and set_package_properties it as type RUNTIME e.g.
https://invent.kde.org/plasma/plasma-disks/-/blob/master/cmake/Findsmartctl.cmake

some further nitpicks:
- _debug.h should actually be superfluous as qdebug can inject context
on its own https://doc.qt.io/qt-5/qtglobal.html#qSetMessagePattern
- Application::formatTime might want to use kformat::formatduration
https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html
- the way static singletons are managed looks a bit old school.
function local statics might be neater
https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables

Single *instance() { static Single s; return &s; }

HS