----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106846/#review20496 -----------------------------------------------------------
Looks good. The DB implementation could use a relationship table with columns <split-id> and <tag-id> which keeps which split is marked with which tag. But I am no DB expert at this point. I have not compiled nor run any of this. The review is simply based on code-reading. kmymoney/dialogs/transactioneditor.h <http://git.reviewboard.kde.org/r/106846/#comment16194> Remove extra blanks kmymoney/mymoney/mymoneytag.h <http://git.reviewboard.kde.org/r/106846/#comment16195> TAB? kmymoney/mymoney/mymoneytag.cpp <http://git.reviewboard.kde.org/r/106846/#comment16196> I think we used QDate::isValid() throughout the application. According to the docs inValid() and isNull() are identical. kmymoney/mymoney/mymoneytag.cpp <http://git.reviewboard.kde.org/r/106846/#comment16197> I would write out visiblefrom/-to and tagcolor only if not invalid since you check for their presence in the XML reader method. kmymoney/mymoney/mymoneytag.cpp <http://git.reviewboard.kde.org/r/106846/#comment16198> What do you want to fix here? Since a MyMoneyTag object does not have a reference to any other object it always returns false as you have written it. kmymoney/mymoney/mymoneytransactionfilter.cpp <http://git.reviewboard.kde.org/r/106846/#comment16199> extra spaces kmymoney/mymoney/mymoneytransactionfilter.cpp <http://git.reviewboard.kde.org/r/106846/#comment16200> extra spaces kmymoney/mymoney/storage/imymoneyserialize.h <http://git.reviewboard.kde.org/r/106846/#comment16201> extra spaces kmymoney/mymoney/storage/mymoneydatabasemgr.h <http://git.reviewboard.kde.org/r/106846/#comment16202> extra spaces kmymoney/mymoney/storage/mymoneydatabasemgr.cpp <http://git.reviewboard.kde.org/r/106846/#comment16205> Why not use a single char here? Suggestion 'G' kmymoney/mymoney/storage/mymoneyseqaccessmgr.h <http://git.reviewboard.kde.org/r/106846/#comment16206> extra spaces kmymoney/mymoney/storage/mymoneyseqaccessmgr.h <http://git.reviewboard.kde.org/r/106846/#comment16207> extra spaces kmymoney/mymoney/storage/mymoneyseqaccessmgr.h <http://git.reviewboard.kde.org/r/106846/#comment16208> extra spaces kmymoney/mymoney/storage/mymoneyseqaccessmgr.h <http://git.reviewboard.kde.org/r/106846/#comment16209> extra spaces kmymoney/mymoney/storage/mymoneyseqaccessmgr.cpp <http://git.reviewboard.kde.org/r/106846/#comment16210> Same here with the one letter code 'G' kmymoney/mymoney/storage/mymoneyseqaccessmgr.cpp <http://git.reviewboard.kde.org/r/106846/#comment16215> better use toUInt() as m_nextTagID is unsigned kmymoney/mymoney/storage/mymoneystorageanon.cpp <http://git.reviewboard.kde.org/r/106846/#comment16211> This is a list of attributes the anonymizer must not change. Since your tags don't show up in a KeyValuePair (KVP) you don't need to touch this here AFAICS. kmymoney/mymoney/storage/mymoneystoragebin.h <http://git.reviewboard.kde.org/r/106846/#comment16212> No need to change this here. Binary storage is deprecated. kmymoney/mymoney/storage/mymoneystoragesql.cpp <http://git.reviewboard.kde.org/r/106846/#comment16213> Fernando can can answer here. kmymoney/widgets/kmymoneymvccombo.h <http://git.reviewboard.kde.org/r/106846/#comment16214> extra space - Thomas Baumgart On Oct. 16, 2012, 11:02 p.m., Alessandro Russo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106846/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2012, 11:02 p.m.) > > > Review request for KMymoney. > > > Description > ------- > > 1) I created and almost completed the Tag tab in KMyMoneyView (KTagsView), > it's similar to the payees tab. > 2) I modified many classes in the storage folder and created a new class > MyMoneyTag similar to MyMoneyPayee, now Tags save and load correctly but not > all tag attribute (visibibleFrom,etc..) are saved/loaded yet. > 3) I changed transactioneditor, transaction and KGlobalLedgerView, now there > is a new widget called Tags in the translationeditor that someway works but > no tags are shown in the ledger yet and no widget is showed when you insert > or edit a transaction directly from the ledger (is this a feature that > somebody uses?). > 4) Missing features: > - multi tags per transaction (only 1 tag can be added at this time) > - no tag selection in reports > - hierarchical tags (I'm rethinking about the real usefulness of this > one... as implementing it will require some work ) > - in the tag tab you can select a color for the tag, this will be used to > show the tag in the ledger with this color, also no visibility check is > made. > - no tags in a split > Bugs: > - in the tag tab if you select a tag and click on the 'New tag' button the > selected tag wil be deleted and replaced with an empy "NewTag" > - in the transaction editor when you edit a transaction with a tag the tag > field is empty, when not in edit mode it show the tag name correctly. > - many more... > > > Diffs > ----- > > kmymoney/dialogs/CMakeLists.txt 9a8d782 > kmymoney/dialogs/ktagreassigndlg.h PRE-CREATION > kmymoney/dialogs/ktagreassigndlg.cpp PRE-CREATION > kmymoney/dialogs/ktagreassigndlgdecl.ui PRE-CREATION > kmymoney/dialogs/transactioneditor.h 2535d96 > kmymoney/dialogs/transactioneditor.cpp 06c7fd1 > kmymoney/kmymoney.h 9482812 > kmymoney/kmymoney.cpp 1ef7400 > kmymoney/kmymoneyui.rc 4294c46 > kmymoney/mymoney/CMakeLists.txt d2917b5 > kmymoney/mymoney/mymoneyfile.h 6c1aa25 > kmymoney/mymoney/mymoneyfile.cpp eef7126 > kmymoney/mymoney/mymoneyobjectcontainer.h 21cd74c > kmymoney/mymoney/mymoneyobjectcontainer.cpp 2e9c496 > kmymoney/mymoney/mymoneysplit.h ded6b4c > kmymoney/mymoney/mymoneysplit.cpp 168d1e6 > kmymoney/mymoney/mymoneytag.h PRE-CREATION > kmymoney/mymoney/mymoneytag.cpp PRE-CREATION > kmymoney/mymoney/mymoneytransactionfilter.h 2a7b85d > kmymoney/mymoney/mymoneytransactionfilter.cpp 6650fef > kmymoney/mymoney/storage/CMakeLists.txt 0495726 > kmymoney/mymoney/storage/imymoneyserialize.h ff49986 > kmymoney/mymoney/storage/imymoneystorage.h 8b980b7 > kmymoney/mymoney/storage/mymoneydatabasemgr.h ab587ed > kmymoney/mymoney/storage/mymoneydatabasemgr.cpp f98a965 > kmymoney/mymoney/storage/mymoneydbdef.h a2eecaf > kmymoney/mymoney/storage/mymoneydbdef.cpp 9fa9781 > kmymoney/mymoney/storage/mymoneyseqaccessmgr.h da67e58 > kmymoney/mymoney/storage/mymoneyseqaccessmgr.cpp 232d6b2 > kmymoney/mymoney/storage/mymoneystorageanon.h 482fc9d > kmymoney/mymoney/storage/mymoneystorageanon.cpp 0e7ebdd > kmymoney/mymoney/storage/mymoneystoragebin.h af2b254 > kmymoney/mymoney/storage/mymoneystoragedump.cpp d36b4bb > kmymoney/mymoney/storage/mymoneystoragesql.h 73875b3 > kmymoney/mymoney/storage/mymoneystoragesql.cpp 90ee818 > kmymoney/mymoney/storage/mymoneystoragexml.h 126cf8c > kmymoney/mymoney/storage/mymoneystoragexml.cpp 88a02df > kmymoney/views/CMakeLists.txt de0272f > kmymoney/views/kgloballedgerview.cpp dfaecbd > kmymoney/views/kmymoneyview.h 99ab398 > kmymoney/views/kmymoneyview.cpp 49dcc85 > kmymoney/views/ktagsview.h PRE-CREATION > kmymoney/views/ktagsview.cpp PRE-CREATION > kmymoney/views/ktagsviewdecl.ui PRE-CREATION > kmymoney/widgets/kmymoney.widgets 857e42c > kmymoney/widgets/kmymoneymvccombo.h 305e8a1 > kmymoney/widgets/kmymoneymvccombo.cpp 42a3687 > kmymoney/widgets/transaction.h a28053e > kmymoney/widgets/transaction.cpp bfaafbf > > Diff: http://git.reviewboard.kde.org/r/106846/diff/ > > > Testing > ------- > > > Thanks, > > Alessandro Russo > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
