----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102175/#review5307 -----------------------------------------------------------
i don't like the idea of this being public API, at least not until we know we need it to be. i also recommend changing the name of the class from EngineInstaller to ComponentInstaller, since it seems more generic than just "Engines" (which itself is ambiguous due to ScriptEngines and DataEngines). plasma/CMakeLists.txt <http://git.reviewboard.kde.org/r/102175/#comment4803> mismatch with if plasma/engineinstaller.cpp <http://git.reviewboard.kde.org/r/102175/#comment4804> so if !force, it shouldn't be added to alreadyPrompted? it may also be worthwhile to remove items from alreadyPrompted once there is success (thus freeing up that memory), assuming packagekit returns this information. - Aaron J. On Aug. 2, 2011, 2:57 a.m., Kevin Kofler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102175/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2011, 2:57 a.m.) > > > Review request for Plasma. > > > Summary > ------- > > This is part of my GSoC 2011 work. Expect more Plasma patches related to this > in the next few days. > > In particular, the currently unused "force" feature of the new API is > intended to be used if the user just installed a widget which explicitly > requires a given engine. (By default, prompts are not repeated within a > session to prevent flooding the user.) > > The implementation is based on PackageKit. It requires PackageKit >= 0.6.16 > and either Apper trunk or the backported patch from > http://pkgs.fedoraproject.org/gitweb/?p=kpackagekit.git;a=blob;f=kpackagekit06-plasma.patch;h=208427aa6cc072164b6a9ba48a30a954657ef892;hb=HEAD > to have any effect. If the requirements are not met, no change will be > noticeable at all, because the asynchronous call will simply fail and any > errors are (intentionally) discarded. (We don't want to annoy the user with a > pointless error dialog if he/she doesn't have PackageKit installed or his/her > version is too old.) > > PackageKit will also only actually find the relevant packages if the > distribution is using RPM and yum (at this time) and if the RPMs in the > repository have been built with my Provides generator. But that shouldn't be > Plasma's problem. Any work in that area needs to happen on the PackageKit or > distro side. Support for GStreamer plugins has been implemented in other > PackageKit backends too, so it should also be doable for Plasma engines. And > if a distro wants to use something entirely different from PackageKit, that's > also possible, this is what the abstract EngineInstaller API is for. > > The API is public because it might be useful outside of libplasma, and it is > quite simple and generic, thus keeping BC should not be a problem. > > > Diffs > ----- > > includes/CMakeLists.txt a967a92 > includes/Plasma/EngineInstaller PRE-CREATION > plasma/CMakeLists.txt ef411df > plasma/dataenginemanager.cpp 988fe76 > plasma/engineinstaller.h PRE-CREATION > plasma/engineinstaller.cpp PRE-CREATION > plasma/scripting/scriptengine.cpp fb8cd1a > > Diff: http://git.reviewboard.kde.org/r/102175/diff > > > Testing > ------- > > Verified that it compiles without errors and that it successfully prompts for > a missing Python script engine on Fedora 15. (The patch is against master > (4.8), but applies without changes to the kdelibs 4.6.5 in Fedora 15, which > is how I tested it.) > > > Thanks, > > Kevin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
