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










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 1st, 2014, 4:15 p.m. CET, <b>Thomas Baumgart</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<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="https://git.reviewboard.kde.org/r/114767/diff/1/?file=228526#file228526line470" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/wizards/newloanwizard/keditloanwizard.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </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; ">const MyMoneyAccount KEditLoanWizard::account(void) const</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">470</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">QList</span><span class="o"><</span><span class="n">MyMoneyInstitution</span><span class="o">></span> <span class="n">list</span> <span class="o">=</span> <span class="n">file</span><span class="o">-></span><span class="n">institutionList</span><span class="p">();</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Think about using "void institutionList(QList<MyMoneyInstitution>& list) const" here. Avoids another copy operation.</pre>
 </blockquote>



 <p>On January 3rd, 2014, 7:40 a.m. CET, <b>Jeremy Whiting</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;">I copied this bit of code directly from KNewAccountDlg.cpp, I'll update my patch, but that code I copied from ought to be fixed also probably.</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes. It's not wrong per se, but new code should use the method I mentioned.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 1st, 2014, 4:15 p.m. CET, <b>Thomas Baumgart</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<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="https://git.reviewboard.kde.org/r/114767/diff/1/?file=228530#file228530line84" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/wizards/newloanwizard/loanattributeswizardpage.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

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



 
 

 <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">84</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="k">if</span> <span class="p">(</span><span class="n">dlg</span><span class="o">-></span><span class="n">exec</span><span class="p">())</span> <span class="p">{</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Better use

 if(dlg->exec() && dlg != 0)

here. See http://blogs.kde.org/node/3919 for details.</pre>
 </blockquote>



 <p>On January 3rd, 2014, 7:40 a.m. CET, <b>Jeremy Whiting</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;">Noted.

I do have one other question about this task. I can't seem to get the institutions to be sorted in the combobox alphabetically.  I set the insertPolicy of the combobox to be alphabetical, yet the entries appear in the order in which they appear in the xml file I guess. The institution list in the KNewAccountDlg is sorted alphabetically, but I don't see what's sorting it. Any ideas?</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Check NewAccountWizard::InstitutionPage::slotLoadWidgets() which contains a qsort() statement. KNewAccountDlg::slotLoadInstitutions(const QString& name) should probably also receive the sorting feature.</pre>
<br />




<p>- Thomas</p>


<br />
<p>On December 31st, 2013, 6:37 p.m. CET, Jeremy Whiting wrote:</p>








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

<div>Review request for KMymoney.</div>
<div>By Jeremy Whiting.</div>


<p style="color: grey;"><i>Updated Dec. 31, 2013, 6:37 p.m.</i></p>







<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=257619">257619</a>


</div>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kmymoney
</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;">Add ability to modify loan institution attribute.

Note this is definitely a work in progress, the QWizardPage title "Attributes" is not best probably, and the page order doesn't make much sense either imo. It probably should go after the radio buttons to ask what you want to change. I'd like to add a fourth option to just modify attributes if that makes sense to everyone, since it's a bit cumbersome to recalculate interest, payoff, etc. just to change the institution the loan belongs to.</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 works here to set and to show the current institution.</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>kmymoney/wizards/newloanwizard/CMakeLists.txt <span style="color: grey">(939d569c2bba56ccfd3c9d6b28f8b90d414b9332)</span></li>

 <li>kmymoney/wizards/newloanwizard/keditloanwizard.cpp <span style="color: grey">(5590a359ffc16c92fc3dc24b6484aac1180b1507)</span></li>

 <li>kmymoney/wizards/newloanwizard/knewloanwizard.h <span style="color: grey">(6b3f049912605f4f9cb0e78ee83b14e2767586b8)</span></li>

 <li>kmymoney/wizards/newloanwizard/knewloanwizarddecl.ui <span style="color: grey">(831d85ce388b51a904c76cd998f54ca6a099b347)</span></li>

 <li>kmymoney/wizards/newloanwizard/loanattributeswizardpage.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>kmymoney/wizards/newloanwizard/loanattributeswizardpage.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>kmymoney/wizards/newloanwizard/loanattributeswizardpagedecl.ui <span style="color: grey">(PRE-CREATION)</span></li>

</ul>

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







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








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