[Kde-pim] Review Request: Enable direct attaching of folders in kmail

[Ingo Klöcker]

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


Coding style: In kdepim we use 2 spaces for indentation.

Please read the KOrganizer Coding Style (http://community.kde.org/KDE_PIM/Development/CodingStyle/Korganizer) and adapt your code to it. In particular, check your usage of whitespace. There are spaces in places where we do not put spaces while in other places spaces are missing. I have noted a small number of such places below.


messagecore/attachmentfromfolder.h
<http://git.reviewboard.kde.org/r/102501/#comment5462>

    This class is a job. Please call it AttachmentFromFolderJob so that someone reading the class name immediately knows that it's a job.



messagecore/attachmentfromfolder.h
<http://git.reviewboard.kde.org/r/102501/#comment5463>

    coding style: add spaces after '(' and before ')'



messagecore/attachmentfromfolder.h
<http://git.reviewboard.kde.org/r/102501/#comment5464>

    coding style: missing spaces



messagecore/attachmentfromfolder.h
<http://git.reviewboard.kde.org/r/102501/#comment5465>

    documentation is missing



messagecore/attachmentfromfolder.h
<http://git.reviewboard.kde.org/r/102501/#comment5466>

    wrong indentation



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5467>

    sort alphabetically (and include KDE headers before Qt headers, but if you sort alphabetically that will anyway be the case)



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5468>

    MessageCore:: is superfluous because of the "using namespace"



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5470>

    coding style: no spaces between method names and opening '('; this applies also to variable initialization (e.g. line 58) and calling of methods/functions everywhere in this file



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5469>

    ... and please make it a QScopedPointer<KZip>. Now that Qt provides nice shared pointer classes we should really use them everywhere instead of raw pointers in order to eliminate every chance of producing a memory leak.



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5471>

    coding style: space after "if" is okay, space after "open" is wrong



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5472>

    coding style: missing spaces after commas
    
    coding style: if you put a space before '+' then also put one after '+'
    
    use char literals instead of single character string literals for any QString- or QByteArray-operations, i.e. use '/' instead of "/" here



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5474>

    Probably use semantic markup instead of \"%1\"



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5473>

    + '/' (see above)



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5476>

    You should check whether the file could be opened.



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5475>

    ditto (semantic markup)



messagecore/attachmentfromfolder.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5477>

    coding style: put spaces around '='


- Ingo


On Aug. 31, 2011, 11:54 p.m., Martin Bednar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102501/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2011, 11:54 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Summary
> -------
> 
> Creates a new class attachmentfromfolder that enables automatic compression (zip) and attachment of folders dropped on the messagecomposer window.
> 
> 
> This addresses bug 42767.
>     http://bugs.kde.org/show_bug.cgi?id=42767
> 
> 
> Diffs
> -----
> 
>   messagecomposer/attachmentcontrollerbase.cpp f8aea5c 
>   messagecore/CMakeLists.txt 821625f 
>   messagecore/attachmentfromfolder.h PRE-CREATION 
>   messagecore/attachmentfromfolder.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102501/diff
> 
> 
> Testing
> -------
> 
> Sent myself an email with a dropped folder attached. The folder contained a .ogg and a text file. The .ogg played correctly, the text file was readable with correct contents.
> 
> 
> Thanks,
> 
> Martin
> 
>

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