On Montag, 6. April 2020 12:32:54 CEST Simon Persson wrote: > Hello! > > > Please help to review kup. > > It is a backup scheduler tightly integrated with plasma (has system > setting kcm, systray plasmoid, kioslave). It supports saving backups > either with bup or with rsync. > > It has been developed outside of KDE for many years and only now is > being incubated. > > I am unsure if it should end up in extragear or some release service > bundle. Perhaps people have an opinion on this? > > https://invent.kde.org/kde/kup > > > Thanks, > > Simon
Hi, I briefly skimmed trough the codebase. Looks all sane to me. A few minor observations: - You may want to look into KConfigXT. It should be able to generate the classes from settings/ from an XML description. - You are using KInit for the daemon executable. We are looking into killing that in the KF6 transition (https://phabricator.kde.org/T12140[1] ) 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. - In https://invent.kde.org/kde/kup/-/blob/master/filedigger/mergedvfsmodel.cpp#L60[2] 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. Cheers Nico -------- [1] https://phabricator.kde.org/T12140 [2] https://invent.kde.org/kde/kup/-/blob/master/filedigger/mergedvfsmodel.cpp#L60
