My first patch has been posted to the reviewboard (http://reviewboard.kde.org/r/3353/) and committed (r1106625). Here's the patch for category includes: http://pastebin.com/wGEgLQMq

@lliehu: That patch looks fine too, and can also be posted to reviewboard to be committed.

bcooksley wrote:I can commit patch 1 for you if you wish, or you can submit it via reviewboard and the PIM developers can commit it.

Hmm... that is unusual. Other than using C casts instead of C++ casts, though that patch is fine. I would suggest posting that one through reviewboard however.

I'll go with the review board I guess.

For patch 2, how's this instead?
http://pastebin.com/Y0Bta90v

That looks better. Please submit that to reviewboard as well.

@Glen Kaukola, lliehu: excellent work so far, the patches have been great.

kdesupport/akonadi
Check for strings used improperly or should be i18n.
http://pastebin.com/mUP4xLVs

kdesupport/akonadi
Check for spelling errors.
http://pastebin.com/B4qZgsfY

My two review requests so far:
http://reviewboard.kde.org/r/3377/
http://reviewboard.kde.org/r/3376/

The post-review command is pretty handy by the way:
http://www.reviewboard.org/docs/manual/ ... st-review/

kdesupport/akonadi
Check for proper include directives.
http://pastebin.com/TYN1catM

kdesupport/akonadi
Check for C++ ctors that should be declared 'explicit'
http://pastebin.com/RE0Qtbuv

I get a foreach warning on this, from kdesupport/akonadi/server/src/handler/delete.cpp:

Code: Select all

bool Delete::deleteRecursive(Collection & col)
{
  Collection::List children = col.children();
  foreach ( Collection child, children ) {
    if ( !deleteRecursive( child ) )
      return false;
  }
  DataStore *db = connection()->storageBackend();
  return db->cleanupCollection( col );
}

Is this something I just need to leave be? Or how would I modify this?

I guess I'm having trouble with the foreach errors in general. Can you explain those a bit? Or point me somewhere that does? Thanks.

All 4 of those patches ( improperly used strings, spelling, proper includes and explicit ctors ) look good, and can be submitted to review board. Excellent work!

For the foreach() issues, that is because the items are not passd as const references, thus causing the object to be copied, and causing more memory to be used and the application to be slower.

That particular foreach() can be solved as follows ( although the const part may need removing due to the delete operation performed on it... )

Code: Select all

foreach( const Collection& child, children )

kdesupport/attica
Not too much stuff so I rolled all the attica stuff into one patch:
http://pastebin.com/7hjdCCWU

kdesupport/automoc
Not much to this one:
http://pastebin.com/VNt4gnNG