<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/102094/">http://git.reviewboard.kde.org/r/102094/</a>
     </td>
    </tr>
   </table>
   <br />









<div>




<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/102094/diff/5/?file=30107#file30107line55" style="color: black; font-weight: bold; text-decoration: underline;">nepomuk/services/writeback/service/writebackjob.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 5)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Nepomuk::WritebackJob::tryNextPlugin()</pre></td>

  </tr>
 </tbody>




 
 



 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">55</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">connect</span><span class="p">(</span><span class="n">p</span><span class="p">,</span><span class="n">SIGNAL</span><span class="p">(</span><span class="n">finished</span><span class="p">(</span><span class="k">this</span><span class="p">)),</span><span class="k">this</span><span class="p">,</span><span class="n">SLOT</span><span class="p">(</span><span class="n">slotWritebackFinished</span><span class="p">(</span><span class="n">WritebackPlugin</span><span class="o">*</span><span class="p">)));</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">finished(this) is wrong. You need the signal signature which involves the type of the parameter, ie.
SIGNAL(finished(Nepomuk::WritebackPlugin*))

And btw: signals and slots should always be fully qualified, ie. including the full namespace.

Maybe you should test your code? ;)</pre>
</div>
<br />

<div>




<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/102094/diff/5/?file=30109#file30109line87" style="color: black; font-weight: bold; text-decoration: underline;">nepomuk/services/writeback/service/writebackservice.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 5)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Nepomuk::WriteBackService::test(const Nepomuk::Resource &amp; resource)</pre></td>

  </tr>
 </tbody>




 
 



 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">87</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">Nepomuk</span><span class="o">::</span><span class="n">WritebackJob</span> <span class="o">*</span><span class="n">job</span><span class="p">;</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please combine the next line into this one to not have an undefined pointer lying around, even if only for one line.

If you get used to never do that you avoid errors down the line.</pre>
</div>
<br />

<div>




<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/102094/diff/5/?file=30109#file30109line107" style="color: black; font-weight: bold; text-decoration: underline;">nepomuk/services/writeback/service/writebackservice.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 5)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Nepomuk::WriteBackService::test(const Nepomuk::Resource &amp; resource)</pre></td>

  </tr>
 </tbody>






 
 

 <tbody>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">94</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">foreach</span><span class="p">(</span><span class="k">const</span> <span class="n">KSharedPtr</span><span class="o">&lt;</span><span class="n">KService</span><span class="o">&gt;&amp;</span> <span class="n">service</span><span class="p">,</span> <span class="n">services</span><span class="p">)</span> <span class="p">{</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">105</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">            <span class="n">foreach</span><span class="p">(</span><span class="k">const</span> <span class="n">KSharedPtr</span><span class="o">&lt;</span><span class="n">KService</span><span class="o">&gt;&amp;</span> <span class="n">service</span><span class="p">,</span> <span class="n">services</span><span class="p">)</span> <span class="p">{</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Why don&#39;t you put all this into a separate method which takes as parameter a KService::List. Then you avoide code duplication.</pre>
</div>
<br />



<p>- Sebastian</p>


<br />
<p>On July 27th, 2011, 7:24 p.m., Smit Shah wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Nepomuk.</div>
<div>By Smit Shah.</div>


<p style="color: grey;"><i>Updated July 27, 2011, 7:24 p.m.</i></p>




<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;">This is my first attempt to write the writebackjob for the writebackservice.</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>nepomuk/services/writeback/lib/writebackplugin.h <span style="color: grey">(6bcdfd3)</span></li>

 <li>nepomuk/services/writeback/lib/writebackplugin.cpp <span style="color: grey">(2a52a31)</span></li>

 <li>nepomuk/services/writeback/service/CMakeLists.txt <span style="color: grey">(949b379)</span></li>

 <li>nepomuk/services/writeback/service/writebackjob.h <span style="color: grey">(e69de29)</span></li>

 <li>nepomuk/services/writeback/service/writebackjob.cpp <span style="color: grey">(e69de29)</span></li>

 <li>nepomuk/services/writeback/service/writebackservice.h <span style="color: grey">(a9821a6)</span></li>

 <li>nepomuk/services/writeback/service/writebackservice.cpp <span style="color: grey">(be5c738)</span></li>

</ul>

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




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








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