D29119: Dolphin: Implement package kit for deb/rpm/pacman service packages

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> CMakeLists.txt:87
> +if(PackageKitQt5_FOUND)
> +    set(PACKAGEKIT_AVAILABLE true)
> +    add_definitions(-DPACKAGEKIT_AVAILABLE=true)

Please call it `HAVE_PACKAGEKIT` instead.

> CMakeLists.txt:88-91
> +    add_definitions(-DPACKAGEKIT_AVAILABLE=true)
> +else()
> +    add_definitions(-DPACKAGEKIT_AVAILABLE=false)
> +endif()

Please create a `config-packagekit.h.cmake` file instead. See `src/CMakeLists.txt` as example:

  configure_file(config-baloo.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-baloo.h)
  configure_file(config-kactivities.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-kactivities.h)

> servicemenuinstaller.cpp:71
> +
> +    const auto getErrorDetails = [=](PackageKit::Transaction::Error, const QString &details) {
> +       fail(details);

Please call it `exitWithError()` or similar instead, since this is what this lambda actually does.

> servicemenuinstaller.cpp:86
> +{
> +    const auto getErrorDetails = [=](PackageKit::Transaction::Error, const QString &details) {
> +        fail(details);

Same here.

> servicemenuinstaller.cpp:109
> +
> +Q_NORETURN void packageKit(bool install, const QString &fileName)
> +{

Same here: please don't use booleans as arguments.

> servicemenuinstaller.cpp:112-113
> +#if PACKAGEKIT_AVAILABLE
> +    QFile file(fileName);
> +    if (!file.exists()) {
> +        fail(i18n("The file does not exist!"));

Please use `QFileInfo::exists()` instead.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D29119

To: alex, #dolphin, ngraham, elvisangelaccio, meven
Cc: cblack, anthonyfieroni, asturmlechner, meven, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200503/e32772a9/attachment.htm>