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

[Kevin Krammer]

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



messagecomposer/attachmentcontrollerbase.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5449>

    most other includes seem to be sorted alphabetically. not sure if this is intended but might be nice anyway



messagecomposer/attachmentcontrollerbase.cpp
<http://git.reviewboard.kde.org/r/102501/#comment5450>

    Better compare on QString level than on ASCII level:
    KMimeType::findByUrl( url ) == QLatin1String( "inode/directory" )



messagecore/CMakeLists.txt
<http://git.reviewboard.kde.org/r/102501/#comment5451>

    also looks like an alphabetically sorted list



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

    I don't think you need that include.
    KUrl is most likely already properly declared by base class header



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

    Our getters are usually just the name of the property, i.e. not getXXX, just XXX.
    And const



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

    include the current class' header first. Our static code checker Krazy checks for that.



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

    Our method names usually start with lower case letters



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

    pass objects by const reference, e.g.
    const QFileInfoList &f
    



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

    mZip for consistency?



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

    initialize mZip to 0



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

    Move declaration of info into loop (allows to use a const ref, avoids copying)
    
    foreach( const QFileInfo &info, f )


- Kevin


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/