[Kde-pim] Review Request 120692: Coding Style for imageprovider.cpp imageprovider.h socialfeeditem.cpp socialfeeditem.h socialnetworkattributes.cpp socialnetworkattributes.h

[Marc Mutz]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120692/#review68931
-----------------------------------------------------------


Who cares about the review of whitespace if the file contains so many functional issues? :)


akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48192>

    This struct should be declared as Q_MOVABLE_TYPE



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48193>

    QPainter p(&roundedImage);



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48194>

    not initialized



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48195>

    QueuedJobHelper is too large for QList. Use QVector.



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48199>

    uninit'ed variable



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48196>

    if (!queuedJobs.empty())



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48202>

    using QList as a queue. Use std::queue, which is the correct container for this.



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48203>

    Dereferening of dynamic_cast result without checking for nullptr.



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48204>

    - value<QUrl>() leads to code duplication. Should use toUrl()
    - unchecked dynamic_cast here, too



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48198>

    - bad variable naming (-Wshadow), leading to ugly this-> qualification in line 139
    - missing const



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48197>

    Repeated re-evaluation of
    
       kiojob->propert(...).value<QUrl>()



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48200>

    should use ctor-init-list



akonadi-socialutils/src/imageprovider.cpp
<https://git.reviewboard.kde.org/r/120692/#comment48201>

    ditto


- Marc Mutz


On Oct. 21, 2014, 5:09 p.m., Guy Maurel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120692/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 5:09 p.m.)
> 
> 
> Review request for KDEPIM-Libraries, Dan Vrátil, Kevin Krammer, and Laurent Montel.
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> Details can be seen at:
>   http://techbase.kde.org/Policies/Kdepim_Coding_Style
> 
> 
> Diffs
> -----
> 
>   akonadi-socialutils/src/imageprovider.h 16c96aa 
>   akonadi-socialutils/src/imageprovider.cpp 430941d 
>   akonadi-socialutils/src/socialfeeditem.h 388e557 
>   akonadi-socialutils/src/socialfeeditem.cpp 67933e4 
>   akonadi-socialutils/src/socialnetworkattributes.h 07c9257 
>   akonadi-socialutils/src/socialnetworkattributes.cpp e8ba3e6 
> 
> Diff: https://git.reviewboard.kde.org/r/120692/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guy Maurel
> 
>

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