[Kde-pim] Review Request: Support for message indicator in KMail

Thomas McGuire mcguire at kde.org
Wed Nov 11 14:52:43 GMT 2009


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

Ship it!


Thanks for submitting this, much better to see patches upstream. Also helps me get an idea what Kubuntu does, and helps understanding bug reports better.

As Ingo said, this patch will not be useful anymore for an Akonadi-ported KMail, as those classes like KMFolder simply don't exist there anymore. I'm fine with having the patch for 4.4. It will probably not be ported to the Akonadi based KMail, unless you do that yourself.

So if you want this patch in KDE, please correct the small issues mentioned below and commit.

Oh, and before I forget it: The patch needs a coding style cleanup, mostly spaces at the inside of parenthesis.




trunk/KDE/kdepim/kmail/kmkernel.h
<http://reviewboard.kde.org/r/2060/#comment2475>

    There already is a public slots section somewhere, re-use that one.



trunk/KDE/kdepim/kmail/kmkernel.cpp
<http://reviewboard.kde.org/r/2060/#comment2474>

    appName can be const



trunk/KDE/kdepim/kmail/kmkernel.cpp
<http://reviewboard.kde.org/r/2060/#comment2473>

    There is also KMSystemTray::showKMail(), I think it would make more sense to use that, and maybe integrate Kontact plugin switching there.


- Thomas


On 2009-11-04 14:33:09, Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2060/
> -----------------------------------------------------------
> 
> (Updated 2009-11-04 14:33:09)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This patch adds optional support for message indicator to KMail. For more information about message indicators, have a look at this blog post: http://agateau.wordpress.com/2009/09/18/indicators-notifications-and-co/
> 
> You need libindicate and libindicate-qt installed to build it. You also need the indicator plasmoid at runtime otherwise you won't see anything interesting.
> - http://launchpad.net/libindicate
> - http://launchpad.net/libindicate-qt
> - http://launchpad.net/plasma-indicatordisplay
> 
> The patch is quite large, it is also available as more incremental steps here if you find this easier to read: http://people.canonical.com/~agateau/indicate/
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/kmail/CMakeLists.txt 1044435 
>   trunk/KDE/kdepim/kmail/config-kmail.h.cmake 1044435 
>   trunk/KDE/kdepim/kmail/configuredialog.cpp 1044435 
>   trunk/KDE/kdepim/kmail/kmail.kcfg.cmake 1044435 
>   trunk/KDE/kdepim/kmail/kmfolder.h 1044435 
>   trunk/KDE/kdepim/kmail/kmfolder.cpp 1044435 
>   trunk/KDE/kdepim/kmail/kmkernel.h 1044435 
>   trunk/KDE/kdepim/kmail/kmkernel.cpp 1044435 
>   trunk/KDE/kdepim/kmail/ui/accountspagereceivingtab.ui 1044435 
> 
> Diff: http://reviewboard.kde.org/r/2060/diff
> 
> 
> Testing
> -------
> 
> This patch has been in use during Kubuntu Karmic development and is now deployed with Kubuntu Karmic version of KMail (kdepim 4.3.2 at the time of this writing).
> 
> 
> Thanks,
> 
> Aurélien
> 
>

_______________________________________________
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/


More information about the kde-pim mailing list