[Kde-pim] Review Request: Fix krazy issue with obsolete QMessageBox::information() call

Michael Leupold lemma at confuego.org
Fri Jul 10 18:32:04 BST 2009



> On 2009-07-09 04:20:53, Thomas McGuire wrote:
> > The patch seems to be missing files, it says /dev/null here.
> 
> Michael Leupold wrote:
>     Sorry, that seems to be a problem with the git diff script I'm using (which puts /dev/null as origin for newly created files). I'm not sure how to actually fix it - downloading the diff works just fine. Is that alright as a solution for you?
> 
> Thomas McGuire wrote:
>     I don't know what you mean by "downloading the diff".
>     But giving me the complete diff for review would be nice :)

I meant you can get the diff using the "download diff" button - albeit only for local viewing. I'll try my best to make it available on reviewboard as well once I'm back home later.


- Michael


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


On 2009-07-08 14:58:30, Michael Leupold wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/957/
> -----------------------------------------------------------
> 
> (Updated 2009-07-08 14:58:30)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This patch replaces a call to QMessageBox::information() with button label arguments. This message is obsolete and it would be better to use KMessageBox or KDialog. Unfortunately this was a little more complicated than I anticipated as the dialog has a special requirement: Default button and cancel button (esc) should be the same ("Ignore"). The easiest way to achieve it (that I found) was creating a separate class inheriting KDialog.
> 
> Further improvement could be made if the MDN mode (eg. return value of requestAdviceOnMDN) was made an enum instead of an int. It does seem to interact with the user's config though so that might not be possible (haven't checked if it pops up anywhere else).
> 
> 
> Diffs
> -----
> 
>   /dev/null 993480 
>   /dev/null 993480 
>   /trunk/KDE/kdepim/kmail/CMakeLists.txt 993480 
>   /trunk/KDE/kdepim/kmail/kmmessage.cpp 993480 
> 
> Diff: http://reviewboard.kde.org/r/957/diff
> 
> 
> Testing
> -------
> 
> Only out-of-KMail testing performed. I hope I correctly reengineered the requestAdviceOnMDN return values.
> 
> 
> Thanks,
> 
> Michael
> 
>

_______________________________________________
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