<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/124633/">https://git.reviewboard.kde.org/r/124633/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On August 6th, 2015, 7:13 a.m. UTC, <b>David Faure</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <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;">If the goal is to query this property for the K menu for instance, aren't you missing an accessor in KService? OK, on the other hand, if this is the only user, then property("...") would do too, but still, compile-time checking is more robust.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">About ksycocathreadtest: I just pushed many fixes for it, does it pass for you now?</p></pre>
 </blockquote>







</blockquote>

<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;">My primary concern was to make sure FormFactors is not lost in conversion. Adding API in KPluginInfo was more a side-effect of this (for symmetry reasons with surrounding code). I don't care so much for an accessor also in KService.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The ksycocathreadtest now succeeds, thanks.</p></pre>
<br />










<p>- Sebastian</p>


<br />
<p>On August 5th, 2015, 10:41 p.m. UTC, 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, Alex Richardson, and David Faure.</div>
<div>By Sebastian Kügler.</div>


<p style="color: grey;"><i>Updated Aug. 5, 2015, 10:41 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kservice
</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;">Parse formfactor in KService and KPluginInfo</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Add an accessor QStringList KPluginInfo::formFactors()</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This corresponds to the X-KDE-FormFactors value in the .desktop file,
and to the "FormFactors" value in the "KPlugin" block of the json
metadata.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We already have this in KPluginMetaData, this patch brings KPluginInfo 
and KService in line with that. Previously, the keys would not be 
recognized, and thus be missing from the KPluginMetaData if created 
through a KPluginInfo or a KService. This patch fixes this inconsistent
behaviour.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also bump the sycoca version.</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 new tests, no problems.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">ksycocathreadtest currently fails, but this is unrelated (also in master without this patch).</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/fakeplugin.desktop <span style="color: grey">(200f023)</span></li>

 <li>autotests/fakeplugin.json <span style="color: grey">(de4aed9)</span></li>

 <li>autotests/kplugininfotest.cpp <span style="color: grey">(d99b92a)</span></li>

 <li>autotests/kservicetest.h <span style="color: grey">(380cf7b)</span></li>

 <li>autotests/kservicetest.cpp <span style="color: grey">(2c71331)</span></li>

 <li>src/services/kplugininfo.h <span style="color: grey">(7b98576)</span></li>

 <li>src/services/kplugininfo.cpp <span style="color: grey">(56dc0b4)</span></li>

 <li>src/services/kservice.cpp <span style="color: grey">(3639a28)</span></li>

 <li>src/services/kservice_p.h <span style="color: grey">(bf59f38)</span></li>

 <li>src/sycoca/ksycoca.cpp <span style="color: grey">(32d1689)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/124633/diff/" style="margin-left: 3em;">View Diff</a></p>






  </td>
 </tr>
</table>







  </div>
 </body>
</html>