D28701: Add KPackage support to KNewStuffCore

This revision was automatically updated to reflect the committed changes. Closed by commit R304:3f38da8a70d8: Add KPackage support to KNewStuffCore (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28701?vs=81859=81868 REVISION DETAIL

D28701: Add KPackage support to KNewStuffCore

mart accepted this revision. This revision is now accepted and ready to land. REPOSITORY R304 KNewStuff BRANCH add-kpackage-support (branched from master) REVISION DETAIL https://phabricator.kde.org/D28701 To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson,

D28701: Add KPackage support to KNewStuffCore

leinir updated this revision to Diff 81859. leinir added a comment. Since we have had a new Frameworks release while this is waiting for the thumbs up, increase the @since to the next version. - Merge branch 'master' into add-kpackage-support - Merge branch 'master' into

D28701: Add KPackage support to KNewStuffCore

leinir added a comment. In D28701#660186 , @mart wrote: > +1 from me. Thanks! :) > does it need to go into next release or can also get in next+1? It really wants to go in soon as possible (since it fixes the whole global

D28701: Add KPackage support to KNewStuffCore

mart added a comment. +1 from me. does it need to go into next release or can also get in next+1? REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D28701 To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, broulik, bshah Cc: alex,

D28701: Add KPackage support to KNewStuffCore

leinir added a comment. Ping team framework and such? (i realise we're all a tiny bit more stressy than usual...) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D28701 To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, broulik, bshah

D28701: Add KPackage support to KNewStuffCore

leinir added inline comments. INLINE COMMENTS > mart wrote in kpackagejob.cpp:55 > is it worth encapsulating it in a runnable? the installation of a package is > already in a separathe thread. Annoyingly, the caller waits for that thread to complete, so the KPackage internals work

D28701: Add KPackage support to KNewStuffCore

mart added inline comments. INLINE COMMENTS > kpackagejob.cpp:55 > + > +class KPackageTask : public QObject, public QRunnable > +{ is it worth encapsulating it in a runnable? the installation of a package is already in a separathe thread. > kpackagejob.cpp:97 > +}); > +

D28701: Add KPackage support to KNewStuffCore

leinir updated this revision to Diff 80862. leinir added a comment. Some documentation and whitespace fixes for Frameworksiness - Fix up a few whitespace errors - Add some documentation to KPackageJob, and a few @since REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE

D28701: Add KPackage support to KNewStuffCore

leinir added a comment. In D28701#653954 , @ngraham wrote: > I mean, GHNS basically *is* a userspace package manager, and things like dependency management and removal of stale files are why package managers exist. :) > > If it needs to be

D28701: Add KPackage support to KNewStuffCore

ngraham added a comment. I mean, GHNS basically *is* a userspace package manager, and things like dependency management and removal of stale files are why package managers exist. :) If it needs to be fixed/implemented in other patches, that's okay. REPOSITORY R304 KNewStuff REVISION

D28701: Add KPackage support to KNewStuffCore

leinir added a comment. In D28701#652898 , @ngraham wrote: > Great. There are still a few more bugs though: > > When you install certain global themes, they ask for authentication so install an SDDM theme. However when you uninstall that

D28701: Add KPackage support to KNewStuffCore

ngraham added a comment. Great. There are still a few more bugs though: When you install certain global themes, they ask for authentication so install an SDDM theme. However when you uninstall that theme, it doesn't request authentication to remove them SDDM theme. So

D28701: Add KPackage support to KNewStuffCore

leinir updated this revision to Diff 80617. leinir added a comment. Thank you to @ngraham for noticing this one! It only really pokes its head out if you have multiple things installed and then try and uninstall one of them - if you only have the one thing installed, it looks very much

D28701: Add KPackage support to KNewStuffCore

leinir added a comment. As i thought, i was indeed holding the KPackage APi incorrectly ;) The culprit is hinted at in the line > org.kde.knewstuff.core: Attempting to perform an installation operation of type 3 on the package "/home/leinir/.local/share/plasma/desktoptheme/Sweet" of

D28701: Add KPackage support to KNewStuffCore

leinir added a comment. In D28701#650616 , @ngraham wrote: > Thanks, the hang is gone now. However I have a new problem: > > 1. Open Global Themes KCM and click new new global themes > 2. Install Sweet KDE and McMojave LAF global themes >

D28701: Add KPackage support to KNewStuffCore

ngraham added a comment. Thanks, the hang is gone now. However I have a new problem: 1. Open Global Themes KCM and click new new global themes 2. Install Sweet KDE and McMojave LAF global themes 3. Close the GHNS dialog 4. Open the dialog again 5. Uninstall the McMojave LAF theme

D28701: Add KPackage support to KNewStuffCore

leinir added reviewers: broulik, bshah. leinir added a comment. Tagging in a couple of people who were in the original chat about doing this integration... :) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D28701 To: leinir, #plasma, #knewstuff, #frameworks,

D28701: Add KPackage support to KNewStuffCore

leinir edited the summary of this revision. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D28701 To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D28701: Add KPackage support to KNewStuffCore

leinir updated this revision to Diff 80289. leinir added a comment. Address @ngraham's (and my own) worry about the synchronous behaviour exhibited by KPackage... Something a bit like this probably wants to go into KPackage itself, perhaps we can consider this after we've done a bit of

D28701: Add KPackage support to KNewStuffCore

leinir added a comment. In D28701#649012 , @ngraham wrote: > This has fixed the issues I was seeing with installation and uninstallation, nice! However I'm not seeing a very long hang when installing certain global themes--for example Sweet KDE

D28701: Add KPackage support to KNewStuffCore

ngraham added a comment. This has fixed the issues I was seeing with installation and uninstallation, nice! However I'm not seeing a very long hang when installing certain global themes--for example Sweet KDE and Layan Look and Feel Theme. It does ultimately work, but the dialog freezes for

D28701: Add KPackage support to KNewStuffCore

leinir added reviewers: ngraham, mart, davidedmundson. leinir added a comment. Tagging in those active in the referenced bug, except for the reporter who doesn't have a phabricator account REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D28701 To: leinir, #plasma,

D28701: Add KPackage support to KNewStuffCore

leinir retitled this revision from "[WIP] Add KPackage support to KNewStuffCore" to "Add KPackage support to KNewStuffCore". leinir edited the summary of this revision. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D28701 To: leinir, #plasma, #knewstuff, #frameworks