[Kde-pim] Re: Review Request: kmail: fix 251009 bug. Send mail doesn't stored in the default send-mail folder if the custom one doesn't exist

[Thomas McGuire]

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


Thanks for the improved version of the patch, a lot better already.
You wrote that you tested that KMail compiles. Does the maildispatcher agent compile as well? Did you test it to see if it actually works? Having two ItemMoveJob's doing the same thing seems a bit fishy to me.


agents/maildispatcher/sendjob.cpp
<http://git.reviewboard.kde.org/r/100269/#comment854>

    Looks like this is also executed in case the sent behavior is to delete the mail. In this case, this code shouldn't be necessary.



agents/maildispatcher/sendjob.cpp
<http://git.reviewboard.kde.org/r/100269/#comment855>

    The starting of the collectionfetchjob should probably happen here in this 'else' branch somewhere.



agents/maildispatcher/sendjob.cpp
<http://git.reviewboard.kde.org/r/100269/#comment859>

    The ItemMoveJob here looks like it is unnecessary now, since it is now done in fetchFinished(), isn't it?



agents/maildispatcher/sendjob.cpp
<http://git.reviewboard.kde.org/r/100269/#comment856>

    What about error handling such as q->setError() etc?



agents/maildispatcher/sendjob.cpp
<http://git.reviewboard.kde.org/r/100269/#comment857>

    I don't think relying on the icon name for something here is a good idea, the user can just change that to something else.



agents/maildispatcher/sendjob.cpp
<http://git.reviewboard.kde.org/r/100269/#comment858>

    Isn't there a connect to the result() signal missing?


- Thomas


On Jan. 18, 2011, 11:43 a.m., Antonis Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100269/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2011, 11:43 a.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Summary
> -------
> 
> Hello
> 
> If you tried to send an email but the custom send-mail folder (settings->configure kmail->identities->modify->advanced) doesn't exist then the kmail doesn't store the send-mail into the default... So with this patch the send-mail is saved in the custom folder (if it exists) otherwise it is saved in the default one...
> 
> 
> This addresses bug 251009.
>     http://bugs.kde.org/show_bug.cgi?id=251009
> 
> 
> Diffs
> -----
> 
>   agents/maildispatcher/sendjob.h 8f0cdc8 
>   agents/maildispatcher/sendjob.cpp 8e1fcc1 
> 
> Diff: http://git.reviewboard.kde.org/r/100269/diff
> 
> 
> Testing
> -------
> 
> The kmail compiles fine and kmail works without any issue
> 
> 
> Thanks,
> 
> Antonis
> 
>

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