<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/121122/">https://git.reviewboard.kde.org/r/121122/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Looks good to me once the open issues have be solved.</p></pre>
<br />
<p>- Alexander Richardson</p>
<br />
<p>On November 14th, 2014, 6:08 nachm. GMT, Sebastian Kügler wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, Plasma, Alexander Richardson, David Faure, and Marco Martin.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated Nov. 14, 2014, 6:08 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kcoreaddons
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This set of patches moves the code to decode and json-serialize from desktoptojson into its own class, used by the KPluginMetaData(QString file) constructor.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">These functions are not exported, desktoptojson directly includes this class. I've done it this way to avoid littering the API, yet removing as much duplicated code from desktoptojson as possible. These functions have gone into their own class, and made static, since they don't carry state, anyway.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The purpose of this patch is to allow the package indexer in kpackage (the split-out framework containing Plasma::Package) to create a correct, KPlugin-compatible cache file from package metadata. This patch allows us to just feed a .desktop file into KPluginMetaData and get back correct deserialized KPluginMetaData objects. It allows us to cut out the KService dependency in KPackage.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In particular, this is split up into the following patches (you can find it in git kcoreaddons[sebas/kpluginmetadata] as well):</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Remove now-duplicated code in desktoptojson</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Use escapeValue, deserializeList and convertToJson from
desktopfileparser.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Move the bits to parse .desktop files into its own class</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This encapsulates the desktop file parseing logic into its own class.
It's fairly self-contained, all static code. This also makes it easier
to share this code with the desktoptojson binary.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Allow to read KPluginMetaData from .desktop files</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Move the logic to create a KPluginMetaData object from desktoptojson
into KPluginMetaData.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This allows to load metadata from a .desktop file, making it possible to
load desktop files without going through KPluginInfo (and thus
KService). This is useful for conversion to json data, which will be
used to index packages in the KPackage framework for faster lookups,
similar to the index proof-of-concept for KPluginTrader.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Add an autotest for loading KPluginMetaData from a .desktop file</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Added an autotest which compares a KPluginMetaData object created from a desktop file. Also used this code from kpackage.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>autotests/data/fakeplugin.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/kpluginmetadatatest.cpp <span style="color: grey">(a1aaf80c8a5e25c2ae031a80fd1f8f96fa5924a0)</span></li>
<li>src/desktoptojson/CMakeLists.txt <span style="color: grey">(c2d2b8780ab6cb34686bc7a43a3b6d4eaeb12b1e)</span></li>
<li>src/desktoptojson/desktoptojson.cpp <span style="color: grey">(3f0cfb129730cf7ba8adbf9c331d967f8a04d4e8)</span></li>
<li>src/lib/CMakeLists.txt <span style="color: grey">(1dc56270ab5157af706b7745cfa88ae11e16184a)</span></li>
<li>src/lib/plugin/desktopfileparser.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/plugin/desktopfileparser.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/plugin/kpluginmetadata.h <span style="color: grey">(67b68a50225e8ac82706caa849fb330567cee1d2)</span></li>
<li>src/lib/plugin/kpluginmetadata.cpp <span style="color: grey">(f3e68c927370ad9212e8dfc07677ec17c52fdbdd)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121122/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>