<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/108489/">http://git.reviewboard.kde.org/r/108489/</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;">Please ship this patch anyway since it is good as it is now (if it matches the current behavior) and we can discuss modifications to the current behavior in subsequent patches.</pre>
<br />
<p>- Cristian</p>
<br />
<p>On January 19th, 2013, 5:08 p.m. UTC, Łukasz Maszczyński wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 Łukasz Maszczyński.</div>
<p style="color: grey;"><i>Updated Jan. 19, 2013, 5:08 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;">Added some tests to characterize application behavior regarding matching an imported transaction with existing/scheduled one.
I'd like to emphasize two important things in this review:
1. I've marked three tests with a comment "revise behavior", I believe they show unwanted behavior. Description of what I think should be expected is inside the test functions, so please comment those parts and let me know if my understanding is (in)correct.
2. I've removed a check, which I believe is superfluous - splitsReferenceSameAccount(), as transaction filter already takes care of filtering off the transactions which refer to a different account. I believe the 4 tests (*_accountMismatch_* and *_multipleAccounts_*) show that this check is not needed, however it was in the code for a long time, only rewritten during recent refactoring.</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;">make test</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/converter/CMakeLists.txt <span style="color: grey">(8433630af638df863dad55e10a3ead60f0b4c7f6)</span></li>
<li>kmymoney/converter/matchfindertest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kmymoney/converter/matchfindertest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kmymoney/converter/transactionmatchfinder.h <span style="color: grey">(5f276a846de4cc72bd2700f2560a64b8ee260217)</span></li>
<li>kmymoney/converter/transactionmatchfinder.cpp <span style="color: grey">(35e72cbc65dde762ed2e11651185afe2c4a14a3c)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108489/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>