[Kde-pim] Review Request: Support for the metadata-over-DBus API in Kmail

[Thomas McGuire]

> On 2009-03-02 06:20:30, Thomas McGuire wrote:
> > Please take a look at Kevin's and my comments.
> > Also, it crashes here, with the patch applied only, haven't figured out yet why.
> > I admit I didn't have time to test the actual functionality yet, will do that later (when it doesn't crash)
> > Sorry for the long review delay.
> > 
> > [KCrash Handler]
> > #5  0xb6a5c1cf in QCoreApplication::notifyInternal (this=0xbfd5e978, receiver=0x824c8c0, event=0xbfd5e6e4) at kernel/qcoreapplication.cpp:593
> > #6  0xb6a5fd75 in QCoreApplication::sendEvent (receiver=0x824c8c0, event=0xbfd5e6e4) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:208
> > #7  0xb6a6ed78 in QObjectPrivate::setParent_helper (this=0x829af90, o=0x824c8c0) at kernel/qobject.cpp:1892
> > #8  0xb6a6ee67 in QObject::setParent (this=0x829b110, parent=0x824c8c0) at kernel/qobject.cpp:1836
> > #9  0xb6a71cb3 in QObject (this=0x829b110, parent=0x824c8c0) at kernel/qobject.cpp:675
> > #10 0xb6a7aee4 in QTimer (this=0x829b110, parent=0x824c8c0) at kernel/qtimer.cpp:130
> > #11 0xb764ca31 in GlobalSettings (this=0x824c8c0) at /media/dev/trunk/src/KDE/kdepim/kmail/globalsettings.cpp:42
> > #12 0xb764cb34 in GlobalSettings::self () at /media/dev/trunk/src/KDE/kdepim/kmail/globalsettings.cpp:33
> > #13 0xb74b59bb in KMKernel (this=0xbfd5e8cc, parent=0x0, name=0x0) at /media/dev/trunk/src/KDE/kdepim/kmail/kmkernel.cpp:138
> > #14 0x0804b4b1 in main (argc=1, argv=0xbfd5ea54) at /media/dev/trunk/src/KDE/kdepim/kmail/main.cpp:127
> 
> Philip Van Hoof wrote:
>     This backtrace doesn't seem to be related to the changes that this patch adds.

No, they don't seem to be related at all, but it is clearly this patch which triggers that, I've tried with and without. I have no clue why at crashes at this place, but I guess I'll try valgrinding it.


> On 2009-03-02 06:20:30, Thomas McGuire wrote:
> > /trunk/KDE/kdepim/kmail/mailmanagerimpl.cpp, line 92
> > <http://reviewboard.kde.org/r/168/diff/4/?file=1271#file1271line92>
> >
> >     foreach is probably easier here, but that's only a style thing.
> >     Otherwise, use const here more
> 
>  wrote:
>      for ( const_iterator it = msg->tagList()->begin(); it != msg->tagList()->end(); ++it ) {
>         QString tag = (*it);
>         predicates.append( "KMail:MessageTag" );
>         values.append( tag );
>       }
> 
>  wrote:
>       for ( KMMessageTagList::const_iterator it = msg->tagList()->begin(); it != msg->tagList()->end(); ++it ) { .. }

Actually, constBegin() and constEnd() as well.
Or simply
foreach( const KMMessageTag &tag, msg->tagList() )


- Thomas


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


On 2009-03-02 08:32:18, Philip Van Hoof wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/168/
> -----------------------------------------------------------
> 
> (Updated 2009-03-02 08:32:18)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Support for the metadata-over-DBus API in Kmail.
> 
> https://bugs.kde.org/show_bug.cgi?id=185331
> 
> 
> This addresses bug https://bugs.kde.org/show_bug.cgi?id=185331.
>     https://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=185331
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/CMakeLists.txt 917233 
>   /trunk/KDE/kdepim/kmail/kmkernel.h 917233 
>   /trunk/KDE/kdepim/kmail/kmkernel.cpp 917233 
>   /trunk/KDE/kdepim/kmail/mailmanagerimpl.h PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/mailmanagerimpl.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/libkdepim/CMakeLists.txt 917233 
>   /trunk/KDE/kdepim/libkdepim/interfaces/org.freedesktop.email.metadata.Manager.xml PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/168/diff
> 
> 
> Testing
> -------
> 
> Tested with this test tool:
> http://live.gnome.org/Evolution/Metadata?action=AttachFile&do=view&target=valaclientsample.vala
> 
> 
> Thanks,
> 
> Philip
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/