> On Aug. 29, 2014, 2 p.m., 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? > > Aaron J. Seigo wrote: > 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. > > Hrvoje Senjan wrote: > >Yes, but I don't think it maters for two reasons > > Can users of Plasma 5.0.x can expect a broken desktop once KF5 5.2 is out > (if this patch goes in)? > > David Edmundson wrote: > Whilst I agree that the frameworks situation sucks, I'm not sure we have > a choice. > > Frameworks 5.2 releases independently of plasma-workspace. At which point > there is a point in time where one can't compile plasma-workspace. > We could make it go via a no-op that does nothing.
if one doesn't recompile it should still work, shouldn't it? Otherwise I'd suggest to wrap the define in a ifndef NO_DEPRECATED block to keep SIC (even if it means that we carry deprecated stuff for the time being, but after all that's the point of deprecation). - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119988/#review65481 ----------------------------------------------------------- On Aug. 29, 2014, 1:51 p.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, 1:51 p.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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
