[Kmymoney-devel] Review Request: show another icon on homepage accounts tables if there are transactions after the last online transaction

[Marko Käning]

> On Jan. 2, 2013, 7:54 p.m., Łukasz Maszczyński wrote:
> > kmymoney/mymoney/mymoneyfiletest.cpp, line 2112
> > <http://git.reviewboard.kde.org/r/107619/diff/5/?file=103701#file103701line2112>
> >
> >     I think that dependency on test #1 could be easily removed - that's the right way to go, whenever it's possible.

So, you want me to remove this dependency?

(If so, I'd have to duplicate the corresponding code for the 2nd test case.)


> On Jan. 2, 2013, 7:54 p.m., Łukasz Maszczyński wrote:
> > kmymoney/mymoney/mymoneyfiletest.h, line 100
> > <http://git.reviewboard.kde.org/r/107619/diff/5/?file=103700#file103700line100>
> >
> >     You extracted a part of testAddAccounts() just fine, but please also update the original method to call AddOneAccount() so that there's no code duplication.

Hmmm, yes you are right, of course, but for now I did not want to mess with all the other test cases and just get my test up and running.

I'll consider this and perhaps replace this function with a AddAccount(QString& acctName) method to be able to create both accounts in testAddAccounts(). :-)


> On Jan. 2, 2013, 7:54 p.m., Łukasz Maszczyński wrote:
> > kmymoney/mymoney/mymoneyfiletest.cpp, line 2128
> > <http://git.reviewboard.kde.org/r/107619/diff/5/?file=103701#file103701line2128>
> >
> >     You use the same harcoded value in several places in the test - this is error prone and may require unnecessary maintenance effort in the future. Use a.id() as you did before.

I just took it from the original test. Will adapt it according to your proposal.


> On Jan. 2, 2013, 7:54 p.m., Łukasz Maszczyński wrote:
> > kmymoney/mymoney/mymoneyfiletest.cpp, lines 2116-2118
> > <http://git.reviewboard.kde.org/r/107619/diff/5/?file=103701#file103701line2116>
> >
> >     If you rename the methods, I think the comment will no longer be needed

OK, see above.


> On Jan. 2, 2013, 7:54 p.m., Łukasz Maszczyński wrote:
> > kmymoney/mymoney/mymoneyfiletest.h, lines 86-87
> > <http://git.reviewboard.kde.org/r/107619/diff/5/?file=103700#file103700line86>
> >
> >     Split is good, now I'd rename the methods to some more meaningful names, e.g. testHasNewerTransaction_noBalanceDifference() and *_withBalanceDifference()

I had it coded like that in the first place, but then decided for the comments, but okay, I think you are right. I'll change it back all over again.


> On Jan. 2, 2013, 7:54 p.m., Łukasz Maszczyński wrote:
> > kmymoney/mymoney/mymoneyfiletest.cpp, lines 2084-2086
> > <http://git.reviewboard.kde.org/r/107619/diff/5/?file=103701#file103701line2084>
> >
> >     If you rename the methods, I think the comment will no longer be needed

OK, see above.


- Marko


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107619/#review24488
-----------------------------------------------------------


On Jan. 1, 2013, 9:13 p.m., Marko Käning wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107619/
> -----------------------------------------------------------
> 
> (Updated Jan. 1, 2013, 9:13 p.m.)
> 
> 
> Review request for KMymoney and Łukasz Maszczyński.
> 
> 
> Description
> -------
> 
> The homepage accounts tables now show a different icon if there are transactions for an account after the last online transaction - highlighting for the user the necessity to download transactions in order to verify the account status.
> 
> 
> Diffs
> -----
> 
>   kmymoney/mymoney/mymoneyfile.h c43977ce7413eee2bc1e0a841fa548314c71e9df 
>   kmymoney/mymoney/mymoneyfile.cpp 6640356d5e07152f8eb4aecff23d62ee8d853dbd 
>   kmymoney/mymoney/mymoneyfiletest.h 5551fa94b6b34022c8e91c0301847d5479e6b1f2 
>   kmymoney/mymoney/mymoneyfiletest.cpp feb9d57ecd7156fc1eea189a905fa797aeae9183 
>   kmymoney/views/khomeview.cpp c79337176b7265cabe15cad4972bc719e797af7c 
> 
> Diff: http://git.reviewboard.kde.org/r/107619/diff/
> 
> 
> Testing
> -------
> 
> Built and ran application and test cases successfully.
> 
> 
> Screenshots
> -----------
> 
> example for the 4 possible cases of online status
>   http://git.reviewboard.kde.org/r/107619/s/948/
> 
> 
> Thanks,
> 
> Marko Käning
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20130102/a4caf628/attachment-0001.html>