
<html>

<head>

<meta http-equiv="content-type" content="text/html; charset=UTF-8">

</head>

<body><p>On Montag, 6. April 2020 12:32:54 CEST Simon Persson wrote:</p>

<p>> Hello!</p>

<p>> </p>

<p>> </p>

<p>> Please help to review kup.</p>

<p>> </p>

<p>> It is a backup scheduler tightly integrated with plasma (has system</p>

<p>> setting kcm, systray plasmoid, kioslave). It supports saving backups</p>

<p>> either with bup or with rsync.</p>

<p>> </p>

<p>> It has been developed outside of KDE for many years and only now is</p>

<p>> being incubated.</p>

<p>> </p>

<p>> I am unsure if it should end up in extragear or some release service</p>

<p>> bundle. Perhaps people have an opinion on this?</p>

<p>> </p>

<p>> https://invent.kde.org/kde/kup</p>

<p>> </p>

<p>> </p>

<p>> Thanks,</p>

<p>> </p>

<p>> Simon</p>

<p> <p>Hi,</p>

<p> <p>I briefly skimmed trough the codebase. Looks all sane to me. A few minor observations:</p>

<p>- You may want to look into KConfigXT. It should be able to generate the classes from settings/ from an XML description.</p>

<p>- You are using KInit for the daemon executable. We are looking into killing that in the KF6 transition (<a href="https://phabricator.kde.org/T12140">https://phabricator.kde.org/T12140</a> ) so consider it deprecated (although it is not offically yet). If you do not care about using kup outside of Plasma you might also consider implementing the daemon part as a kded module.</p>

<p>- In <a href="https://invent.kde.org/kde/kup/-/blob/master/filedigger/mergedvfsmodel.cpp#L60">https://invent.kde.org/kde/kup/-/blob/master/filedigger/mergedvfsmodel.cpp#L60</a> please check if instead of calling loadMimeTypeIcon simply returning the iconname is enough, or return QIcon::fromTheme(name). loadMimeTypeIcon looks like a likely candidate for deprecation/removal to me. That would also get rid of the KIconThemes dependency.</p>

<p> <p>Cheers</p>

<p> <p>Nico</p>

<p> </p>

<p> </body>

</html>