D29123: WIP BUG: 420312. Do not mark entry as uninstalled if uninstallation script failed

[Dan Leinir Turthra Jensen]

leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  The reporting side of this seems based on a misunderstanding of what the UI-less Core is supposed to be doing... The conceptual intention in general isn't bad, but it needs a bit of work. Thanks for spotting it, too :)

INLINE COMMENTS

> CMakeLists.txt:71
>      KF5::CoreAddons
> +    KF5::WidgetsAddons         # KMessageBox error messages
>      Qt5::Xml

No, that's what the Question system is for. No widget stuff in Core, thanks :)

> installation.cpp:631
> +                    // can delete the files manually
> +                    entry.setStatus(KNS3::Entry::Installed);
> +                    KMessageBox::error(nullptr, err);

If you are changing the status, you need to also emit entryChanged, otherwise the cache will be inconsistent

> installation.cpp:632
> +                    entry.setStatus(KNS3::Entry::Installed);
> +                    KMessageBox::error(nullptr, err);
> +                    return;

As you are already issuing the signal with the error, intercept that instead. Don't spawn widgets from Core, that adds a widget dependency to the Qtquick module.

> installation.cpp:653
> -
> -    emit signalEntryChanged(entry);
>  }

Unless you report the entry as changed, the cache will not be updated and the entire reporting side will fall down. Please put that line back :)

REPOSITORY
  R304 KNewStuff

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

To: alex, #knewstuff, meven, ngraham, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200423/16fb2269/attachment-0001.html>