[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    [Kde-pim] Re: Review Request: Output error message when saving an
From:       Ingo_Klöcker <kloecker () kde ! org>
Date:       2010-12-12 17:05:29
Message-ID: 20101212170529.4042.77499 () vidsolbach ! de
[Download RAW message or body]


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


In general, the patch looks good. Thanks for fixing this bug.

I'm not sure it makes much sense to differentiate the two cases bytesWritten == -1 \
and -1 < bytesWritten < data.size() duplicating quite some code. I suggest combining \
both.

Your patch introduces a new string. Therefore you cannot commit now since we are in \
string freeze for KDE 4.6. Combining both cases and using the old error message would \
solve this.

Please fix the indentation of the code inside the if-clause while you are touching \
the surrounding code. In kdepim we use 2 spaces.


/trunk/KDE/kdepim/messageviewer/util.cpp
<http://svn.reviewboard.kde.org/r/6104/#comment10081>

    Please move this line further down to where it's first needed.



/trunk/KDE/kdepim/messageviewer/util.cpp
<http://svn.reviewboard.kde.org/r/6104/#comment10082>

    I'd prefer you to split this in two lines. In fact, you can combine the \
assignment with the declaration of bytesWritten and then make bytesWritten const.



/trunk/KDE/kdepim/messageviewer/util.cpp
<http://svn.reviewboard.kde.org/r/6104/#comment10083>

    There should probably be a <br> after </filename>


- Ingo


On 2010-12-12 16:43:22, George  Metaxas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6104/
> -----------------------------------------------------------
> 
> (Updated 2010-12-12 16:43:22)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> As discussed in Bug 127981, when one attempts to save an attachment to a disk which \
> has some space available, but not enough to fit the attachment, KMail will not \
> output an error message informing the user about this. Instead, the attachment will \
> be partially saved and the user will be oblivious to what really happened. This \
> patch will show an error message to the user and will also remove the partially \
> written file. KMail does indeed correctly inform the user if the disk has no space \
> left, but it leaves behind a zero-sized file. This patch also removes this \
> unnecessary file. 
> 
> This addresses bug 127981.
> https://bugs.kde.org/show_bug.cgi?id=127981
> 
> 
> Diffs
> -----
> 
> /trunk/KDE/kdepim/messageviewer/util.cpp 1205704 
> 
> Diff: http://svn.reviewboard.kde.org/r/6104/diff
> 
> 
> Testing
> -------
> 
> Tested saving an attachment with:
> - No space left
> - Some space left, but not enough to fit the attachment
> - Enough space to fit the attachment
> 
> 
> Thanks,
> 
> George
> 
> 

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


[prev in list] [next in list] [prev in thread] [next in thread]