<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/100337/">http://git.reviewboard.kde.org/r/100337/</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;">Looks good to me.</pre>
 <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/100337/diff/5/?file=6525#file6525line65" style="color: black; font-weight: bold; text-decoration: underline;">idle/idle-account-ui.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; ">AbstractAccountParametersWidget *IdleAccountUi::mainOptionsWidget(</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">65</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">Tp</span><span class="o">::</span><span class="n">ProtocolParameterList</span> <span class="n">parameters</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;">These should be const references too.

However, this isn&#39;t your fault. It&#39;s the library which is wrong.</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/100337/diff/5/?file=6525#file6525line80" style="color: black; font-weight: bold; text-decoration: underline;">idle/idle-account-ui.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; ">AbstractAccountParametersWidget *IdleAccountUi::mainOptionsWidget(</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">80</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">Tp</span><span class="o">::</span><span class="n">ProtocolParameterList</span> <span class="n">parameters</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;">Same as above.</pre>
</div>
<br />



<p>- David</p>


<br />
<p>On January 9th, 2011, 5:36 p.m., Lasse Liehu wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/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 Telepathy.</div>
<div>By Lasse Liehu.</div>


<p style="color: grey;"><i>Updated Jan. 9, 2011, 5:36 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;">A way to select an IRC network from a set of presets isn&#39;t yet done. Does Telepathy give anything here?

Also there isn&#39;t any hint for the user what &quot;Username&quot; means. I&#39;m still wondering if it being empty means &quot;use current local system username as one&quot; or &quot;use nick as one&quot; and a set value to use that value as one. I guess it should default to empty anyhow. Empathy doesn&#39;t seem to use the parameter in any way.

There isn&#39;t an error message shown if parameter validation fails, but it wouldn&#39;t even show if someone does &quot;Enable/Disable the &quot;next&quot; (or finish button) depending on whether required parameters are filled in.&quot; (as at http://community.kde.org/index.php?title=Real-Time_Communication_and_Collaboration/Components/Accounts_KCM_Tasks ).</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;">It compiles and the plugin loads.
Tested creating and editing an account. All supported parameters load and save correctly and Empathy can use the created account.
Validation (just testing if server and nick are non-empty) works.</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>CMakeLists.txt <span style="color: grey">(4a102d94a28c5ac927039ed63acc10c08b490ad4)</span></li>

 <li>idle/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/Messages.sh <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/advanced-options-widget.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/advanced-options-widget.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/advanced-options-widget.ui <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/idle-account-ui-plugin.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/idle-account-ui-plugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/idle-account-ui.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/idle-account-ui.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/kcmtelepathyaccounts_plugin_idle.desktop <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/main-options-widget.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/main-options-widget.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>idle/main-options-widget.ui <span style="color: grey">(PRE-CREATION)</span></li>

</ul>

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




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








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