|
»
»
Review Request: Support for message indicator in KMail
|
View:
New views
8 Messages
—
Rating Filter:
Alert me
|
 |
Review Request: Support for message indicator in KMail
by Bugzilla from agateau@kde.org
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2060/ ----------------------------------------------------------- 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
|
|
Re: Review Request: Support for message indicator in KMail
by Ingo Klöcker
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2060/#review2931 ----------------------------------------------------------- As much as I would like this patch to go (note: I didn't review it, but I think the feature is useful) in I'm not sure whether it's worth it because: a) In view of Akonadi your patch needs to be rewritten based on Akonadi. The message indicator stuff does not belong into KMail. Instead you should implement it as message indicator Akonadi agent. b) Porting KMail to Akonadi happens on a work branch. Your patch would increase the difference between trunk and this work branch. Consequently, it would make merging of bug fixes from trunk to the Akonadi-port work branch a bit more difficult. In the end Thomas needs to decide. - Ingo 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
|
|
Re: Review Request: Support for message indicator in KMail
by Bugzilla from mcguire@kde.org
::
Rate this Message:
----------------------------------------------------------- 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
|
|
Re: Review Request: Support for message indicator in KMail
by Bugzilla from agateau@kde.org
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2060/ ----------------------------------------------------------- (Updated 2009-11-12 23:36:34.757515) Review request for KDE PIM. Changes ------- Fixed some of the coding style issues. About using KMSystemTray::showKMail(), would it make sense to move showKMail() and hideKMail() to KMKernel? 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 (updated) ----- trunk/KDE/kdepim/kmail/CMakeLists.txt 1048189 trunk/KDE/kdepim/kmail/config-kmail.h.cmake 1048189 trunk/KDE/kdepim/kmail/configuredialog.cpp 1048189 trunk/KDE/kdepim/kmail/kmail.kcfg.cmake 1048189 trunk/KDE/kdepim/kmail/kmfolder.h 1048189 trunk/KDE/kdepim/kmail/kmfolder.cpp 1048189 trunk/KDE/kdepim/kmail/kmkernel.h 1048189 trunk/KDE/kdepim/kmail/kmkernel.cpp 1048189 trunk/KDE/kdepim/kmail/ui/accountspagereceivingtab.ui 1048189 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
|
|
Re: Review Request: Support for message indicator in KMail
by Bugzilla from mcguire@kde.org
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2060/#review3087 ----------------------------------------------------------- Ship it! > Would it make sense to move showKMail() and hideKMail() to KMKernel? Yeah, would probably make sense, can't think of a better place right now. Please make that change and the style fix change and commit. - Thomas On 2009-11-12 23:36:34, Aurélien Gâteau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/2060/ > ----------------------------------------------------------- > > (Updated 2009-11-12 23:36:34) > > > 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 1048189 > trunk/KDE/kdepim/kmail/config-kmail.h.cmake 1048189 > trunk/KDE/kdepim/kmail/configuredialog.cpp 1048189 > trunk/KDE/kdepim/kmail/kmail.kcfg.cmake 1048189 > trunk/KDE/kdepim/kmail/kmfolder.h 1048189 > trunk/KDE/kdepim/kmail/kmfolder.cpp 1048189 > trunk/KDE/kdepim/kmail/kmkernel.h 1048189 > trunk/KDE/kdepim/kmail/kmkernel.cpp 1048189 > trunk/KDE/kdepim/kmail/ui/accountspagereceivingtab.ui 1048189 > > 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
|
|
Re: Review Request: Support for message indicator in KMail
by Bugzilla from mcguire@kde.org
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2060/#review3086 ----------------------------------------------------------- Ship it! > Would it make sense to move showKMail() and hideKMail() to KMKernel? Yeah, would probably make sense, can't think of a better place right now. Please make that change and the style fix change and commit. trunk/KDE/kdepim/kmail/kmfolder.cpp <http://reviewboard.kde.org/r/2060/#comment2515> Hey, there are still missing spaces here and in the next 2 function definitions - Thomas On 2009-11-12 23:36:34, Aurélien Gâteau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/2060/ > ----------------------------------------------------------- > > (Updated 2009-11-12 23:36:34) > > > 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 1048189 > trunk/KDE/kdepim/kmail/config-kmail.h.cmake 1048189 > trunk/KDE/kdepim/kmail/configuredialog.cpp 1048189 > trunk/KDE/kdepim/kmail/kmail.kcfg.cmake 1048189 > trunk/KDE/kdepim/kmail/kmfolder.h 1048189 > trunk/KDE/kdepim/kmail/kmfolder.cpp 1048189 > trunk/KDE/kdepim/kmail/kmkernel.h 1048189 > trunk/KDE/kdepim/kmail/kmkernel.cpp 1048189 > trunk/KDE/kdepim/kmail/ui/accountspagereceivingtab.ui 1048189 > > 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
|
|
Re: Review Request: Support for message indicator in KMail
by Bugzilla from agateau@kde.org
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2060/ ----------------------------------------------------------- (Updated 2009-11-16 05:58:12.660540) Review request for KDE PIM. Changes ------- Added remaining white spaces and moved main window handling from KMSystemTray to KMKernel. The patch is quite large, so I decided to post it here before actually committing it. 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 (updated) ----- trunk/KDE/kdepim/kmail/CMakeLists.txt 1048189 trunk/KDE/kdepim/kmail/config-kmail.h.cmake 1048189 trunk/KDE/kdepim/kmail/configuredialog.cpp 1048189 trunk/KDE/kdepim/kmail/kmail.kcfg.cmake 1048189 trunk/KDE/kdepim/kmail/kmfolder.h 1048189 trunk/KDE/kdepim/kmail/kmfolder.cpp 1048189 trunk/KDE/kdepim/kmail/kmkernel.h 1048189 trunk/KDE/kdepim/kmail/kmkernel.cpp 1048189 trunk/KDE/kdepim/kmail/kmsystemtray.h 1048189 trunk/KDE/kdepim/kmail/kmsystemtray.cpp 1048189 trunk/KDE/kdepim/kmail/ui/accountspagereceivingtab.ui 1048189 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
|
|
Re: Review Request: Support for message indicator in KMail
by Bugzilla from mcguire@kde.org
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2060/#review3123 ----------------------------------------------------------- Ship it! Seems fine, please commit. - Thomas On 2009-11-16 05:58:12, Aurélien Gâteau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/2060/ > ----------------------------------------------------------- > > (Updated 2009-11-16 05:58:12) > > > 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 1048189 > trunk/KDE/kdepim/kmail/config-kmail.h.cmake 1048189 > trunk/KDE/kdepim/kmail/configuredialog.cpp 1048189 > trunk/KDE/kdepim/kmail/kmail.kcfg.cmake 1048189 > trunk/KDE/kdepim/kmail/kmfolder.h 1048189 > trunk/KDE/kdepim/kmail/kmfolder.cpp 1048189 > trunk/KDE/kdepim/kmail/kmkernel.h 1048189 > trunk/KDE/kdepim/kmail/kmkernel.cpp 1048189 > trunk/KDE/kdepim/kmail/kmsystemtray.h 1048189 > trunk/KDE/kdepim/kmail/kmsystemtray.cpp 1048189 > trunk/KDE/kdepim/kmail/ui/accountspagereceivingtab.ui 1048189 > > 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@... https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/ |
| Free embeddable forum powered by Nabble | Forum Help |
