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

List:       kde-pim
Subject:    Re: [Kde-pim] Review Request: Enable direct attaching of folders in
From:       "Kevin Krammer" <kevin.krammer () gmx ! at>
Date:       2011-09-01 8:03:29
Message-ID: 20110901080329.4329.84925 () vidsolbach ! de
[Download RAW message or body]


-----------------------------------------------------------
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@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]