Review Request 119722: Added Emails Plugin

Martin Klapetek martin.klapetek at gmail.com
Tue Aug 12 14:46:14 UTC 2014


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


After this round of review I'm happy to get this in \o/


CMakeLists.txt
<https://git.reviewboard.kde.org/r/119722/#comment44985>

    I'm not sure if we want to hard-depend on Baloo now, this should become optional...I'll discuss it with David here and post back



src/widgets/personemailview.cpp
<https://git.reviewboard.kde.org/r/119722/#comment44978>

    It's fine if you leave this widget in, you can just delete the widget you add below --> retrieve the last widget in the layout and delete that
    
    Or store the widget you get from d->emails->createDetailsWidget as a class variable and delete it direclty



src/widgets/personemailview.cpp
<https://git.reviewboard.kde.org/r/119722/#comment44977>

    for QObjects there's qobject_cast<>(), use that instead



src/widgets/plugins/emaillistmodel.cpp
<https://git.reviewboard.kde.org/r/119722/#comment44979>

    This should keep the Q_UNUSED(parent) if you're not using it



src/widgets/plugins/emaillistmodel.cpp
<https://git.reviewboard.kde.org/r/119722/#comment44980>

    Remove this, this should be done by the delegate
    
    If the delegate does not work properly, you might have the bodyRect with wrong size



src/widgets/plugins/emaillistviewdelegate.cpp
<https://git.reviewboard.kde.org/r/119722/#comment44983>

    Remove empty line



src/widgets/plugins/emails.h
<https://git.reviewboard.kde.org/r/119722/#comment44982>

    Fill the var names here



src/widgets/plugins/emails.cpp
<https://git.reviewboard.kde.org/r/119722/#comment44981>

    remove the "struct" keyword



src/widgets/plugins/emails.cpp
<https://git.reviewboard.kde.org/r/119722/#comment44984>

    Remove empty line


- Martin Klapetek


On Aug. 12, 2014, 3:29 p.m., Nilesh Suthar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119722/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2014, 3:29 p.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Repository: libkpeople
> 
> 
> Description
> -------
> 
> Email are listed in QListview and fetch with the help of baloo
> To be tested with http://quickgit.kde.org/?p=scratch%2Fnileshsuthar%2Fkpeople.git
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt b599284 
>   src/widgets/CMakeLists.txt f637838 
>   src/widgets/personemailview.h PRE-CREATION 
>   src/widgets/personemailview.cpp PRE-CREATION 
>   src/widgets/plugins/emaillistmodel.h PRE-CREATION 
>   src/widgets/plugins/emaillistmodel.cpp PRE-CREATION 
>   src/widgets/plugins/emaillistviewdelegate.h PRE-CREATION 
>   src/widgets/plugins/emaillistviewdelegate.cpp PRE-CREATION 
>   src/widgets/plugins/emails.h PRE-CREATION 
>   src/widgets/plugins/emails.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119722/diff/
> 
> 
> Testing
> -------
> 
> Working as needed
> 
> 
> Thanks,
> 
> Nilesh Suthar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140812/808a1a28/attachment.html>


More information about the KDE-Telepathy mailing list