Review Request 119988: Package structure cleanups

[Aaron J. Seigo]

> On Aug. 29, 2014, noon, David Edmundson wrote:
> > src/plasma/packagestructure.h, line 99
> > <https://git.reviewboard.kde.org/r/119988/diff/1/?file=308188#file308188line99>
> >
> >     would removing a #define count as a SIC?

Yes, but I don't think it maters for two reasons:

1) DataEngine also had both forms but only one actually currently works; so it is on the face of it *broken* .. removing that is the same as this change at the end of the day
2) There are no Plasma 5 PackageStructure plugins out there except the share dataengine in plasma-workspace (which itself was done in a very odd manner)

Neither reason is particularly *good* from the perspective of strict policy adherence, but they demonstrate that it is a harmless violation. May as well get it right imho.

I'd add a third consideration as well: It is quite evident that plasma-framework was release before it was ready. The plugin loading is entirely inconsistent with DataEngines using JSON and the Qt5 plugin loading an the rest of the plugins not. Some of the plugins are obviously not being used at all due to changes brought about with QML2 and that probably contributed to the lack of attention. However, PluginLoader quite obviously went through a partial refactoring that was never completed. It's a little brash to commit to source and binary compatibility when things are in that shape.


- Aaron J.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119988/#review65481
-----------------------------------------------------------


On Aug. 29, 2014, 11:51 a.m., Aaron J. Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119988/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 11:51 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> This patch set does the following:
> 
> * tidy up the data engine plugin loading code
> * make PackageStructure plugins use the json method as with DataEngines
> * remove ShellPackage; it moves to live with plasmashell (review #119989)
> 
> The goal here is to get rid of the plasmaquick library as much as possible. It was unnecessary in the first place since PackageStructure supports plugins. The only potentially controversial change here is to move PackageStructure to use the json-based plugin loading. That seems to be the more modern approach, but plugin loading in libplasma is currently a mix of the old and the new. As PackageStructure changed API in plasma-framework, meaning any existing plugins from 4.x would need updating anyways, this seems a safe enough change to make as it should impact exactly zero plugins out there currently.
> 
> 
> Diffs
> -----
> 
>   src/plasma/packagestructure.h fb32c22a6e7df1528b3d7a5b30b94c60a85a93e3 
>   src/plasma/pluginloader.cpp d2ba5ca2d3a96fe6f1ce26be41df3b0954b924df 
>   src/plasma/private/packages.cpp 5eb6f0021392257634dfd958c940b2945989e48b 
>   src/plasma/private/packages_p.h 0833a4ed1b5704efffccade5e52589878e8b4957 
>   src/plasmaquick/CMakeLists.txt 1ed7c67efcba0e6dbef1ff929b176090786503de 
>   src/plasmaquick/private/packages.h 7498832d0537611903c13e544db6486bab163dd3 
>   src/plasmaquick/private/packages.cpp 52758482230d271712e4bb3b6d33f8fdeaa848a8 
>   src/plasmaquick/shellpluginloader.h 6c56e5f7b269c3af7587a58cbe104468a2c679c4 
>   src/plasmaquick/shellpluginloader.cpp 2824760e6f64a694bd14b46d2f80151304e3e4d3 
>   src/plasma/dataengine.h d87a6f8361c892a249374a5c9da7e84836195156 
>   src/plasma/package.cpp 6ad332167bb83c2f794f9f5d059e9f369ad33841 
> 
> Diff: https://git.reviewboard.kde.org/r/119988/diff/
> 
> 
> Testing
> -------
> 
> Ran a full Plasma Desktop session.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140829/55085890/attachment-0001.html>