D15177: Emit signals when a package is installed/uninstalled

2018-09-13 Thread David Edmundson
This revision was automatically updated to reflect the committed changes.
Closed by commit R290:b42973d964df: Emit signals when a package is 
installed/uninstalled (authored by davidedmundson).

REPOSITORY
  R290 KPackage

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15177?vs=40725&id=41523

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

AFFECTED FILES
  CMakeLists.txt
  src/kpackage/CMakeLists.txt
  src/kpackage/private/packagejob.cpp

To: davidedmundson, #plasma, broulik
Cc: broulik, Zren, kde-frameworks-devel, michaelh, ngraham, bruns


D15177: Emit signals when a package is installed/uninstalled

2018-09-10 Thread David Edmundson
davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in packagejob.cpp:90
> Can this be invalid or does `install` already check for validity?

bool PackageJobThread::installPackage(const QString &src, const QString &dest, 
OperationType operation)
{

...

  if (!meta.isValid()) {
qCDebug(KPACKAGE_LOG) << "No metadata file in package" << src << path;
d->errorMessage = i18n("No metadata file in package: %1", src);
d->errorCode = Package::JobError::MetadataFileMissingError;
return false;
}

REPOSITORY
  R290 KPackage

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

To: davidedmundson, #plasma
Cc: broulik, Zren, kde-frameworks-devel, michaelh, ngraham, bruns


D15177: Emit signals when a package is installed/uninstalled

2018-09-10 Thread Kai Uwe Broulik
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R290 KPackage

BRANCH
  master

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

To: davidedmundson, #plasma, broulik
Cc: broulik, Zren, kde-frameworks-devel, michaelh, ngraham, bruns


D15177: Emit signals when a package is installed/uninstalled

2018-09-10 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> packagejob.cpp:90
> +if (ok) {
> +const QString pluginId = d->package->metadata().pluginId();
> +const QStringList serviceTypes = 
> d->package->metadata().serviceTypes();

Can this be invalid or does `install` already check for validity?

REPOSITORY
  R290 KPackage

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

To: davidedmundson, #plasma
Cc: broulik, Zren, kde-frameworks-devel, michaelh, ngraham, bruns


D15177: Emit signals when a package is installed/uninstalled

2018-08-31 Thread David Edmundson
davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in packagejob.cpp:93
> Can `packageType` contain characters not allowed in DBus?

Technically, yes, it's a string in the metadata of the package type so could.

By convention format X-KDE-ServiceTypes is always in the format Foo/Blah and 
generally we control the package structures.

REPOSITORY
  R290 KPackage

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

To: davidedmundson, #plasma
Cc: broulik, Zren, kde-frameworks-devel, michaelh, ngraham, bruns


D15177: Emit signals when a package is installed/uninstalled

2018-08-31 Thread Kai Uwe Broulik
broulik added a comment.


  +1

INLINE COMMENTS

> packagejob.cpp:93
> +for (auto packageType: serviceTypes) {
> +auto msg = 
> QDBusMessage::createSignal(QStringLiteral("/KPackage/") + packageType, 
> QStringLiteral("org.kde.plasma.kpackage"), 
> QStringLiteral("packageInstalled"));
> +msg.setArguments({pluginId});

Can `packageType` contain characters not allowed in DBus?

REPOSITORY
  R290 KPackage

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

To: davidedmundson, #plasma
Cc: broulik, Zren, kde-frameworks-devel, michaelh, ngraham, bruns


D15177: Emit signals when a package is installed/uninstalled

2018-08-30 Thread Chris Holland
Zren added a dependent revision: D12040: Add wallpaperplugin.knsrc + QML 
function to open GHNS dialog.

REPOSITORY
  R290 KPackage

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

To: davidedmundson, #plasma
Cc: Zren, kde-frameworks-devel, michaelh, ngraham, bruns


D15177: Emit signals when a package is installed/uninstalled

2018-08-30 Thread David Edmundson
davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  This is needed for situations such as Plasma reloading the wallpaper
  plugins when one is installed.
  
  We want something with more granularity than arbitrarily reloading when
  the KNS dialog closes, and less horrible than file watchers.
  
  The service type (for example Plasma/Wallpaper) makes up part of the
  path for optimal filtering.

TEST PLAN
  Ran dbus-monitor and kpacktool5 -r / -i

REPOSITORY
  R290 KPackage

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  src/kpackage/CMakeLists.txt
  src/kpackage/private/packagejob.cpp

To: davidedmundson, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns