[Kde-pim] Review Request: properly honor resource when applying filters to new emails
Ingo Klöcker
kloecker at kde.org
Wed Sep 14 21:37:49 BST 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102446/#review6517
-----------------------------------------------------------
Ship it!
Apart from the below remark the patch looks good.
mailcommon/filtermanager.cpp
<http://git.reviewboard.kde.org/r/102446/#comment5771>
It probably doesn't matter that much, but since always the same resource will be used it might make sense to determine the resource just once (before the for-loop) instead of for each filter in the for-loop.
- Ingo
On Sept. 14, 2011, 12:16 p.m., Milian Wolff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102446/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2011, 12:16 p.m.)
>
>
> Review request for KDEPIM.
>
>
> Summary
> -------
>
> This is most probably just a dirty "get it working" patch, but I want to post it here for review. First the problem it solves:
>
> When starting kmail, it will apply all filters to all new emails without honoring the "apply this filter to incoming messages: from checked accounts only" setting.
>
> The easiest way to notice is by having a "spamc" or spamassassin (or similar) filter that takes a long time and have that apply to only a very specific account. Then, when a different account gets new email you should see a load spike / spamc processes even though there should be none.
>
> Imo the whole design of FilterManager::process's function signature is pretty bad. Shouldn't/couldn't the optional stuff be removed and item.parentCollection().resource() be used instead?
>
>
> Diffs
> -----
>
> mailcommon/filtermanager.h 68e162e
> mailcommon/filtermanager.cpp 7d0372e
>
> Diff: http://git.reviewboard.kde.org/r/102446/diff
>
>
> Testing
> -------
>
> ran kmail2 for ~1 day. it finally worked like it should
>
>
> Thanks,
>
> Milian
>
>
_______________________________________________
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