Review Request 108498: KTp::ContactInfoDialog

[David Edmundson]

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

Ship it!


Very important fix needed (comment #2)

Otherwise very nice.




KTp/contact-info-dialog.h
<http://git.reviewboard.kde.org/r/108498/#comment19724>

    namespace the #ifdef with KTP_  so KTP_CONTACTINFODIALOG_H
    
    (I know most the code doesn't have that right now, I'm trying to slowly fix it throughout :) )



KTp/contact-info-dialog.cpp
<http://git.reviewboard.kde.org/r/108498/#comment19983>

    This is never initialised.
    
    and you even check d->clearAvatarButton at some point :S
    
    I think if you had a non-debug build this would be crashing constantly. (debug builds always init everything to 0)
    
    may be safer to just set all these to 0 in the private constructor like you do with the other avatarButton.


- David Edmundson


On Jan. 20, 2013, 2 a.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108498/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2013, 2 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> KTp::ContactInfoDialog is a dialog very similar to the contact info dialog in contact list, but it displays more detailed information, like full name, homepage, birthday etc.
> 
> I moved the dialog to ktp-common-internals, so that it can be shared between contactlist and kded module - see bug https://bugs.kde.org/show_bug.cgi?id=298764.
> 
> The dialog also supports editing mode (see screenshots), when you open the dialog with "yourself" (that's contact == account->connection->selfContact). This feature could be used in KCM for instance. Unfortunately when I was almost finished, I found out, that TelepathyQt4 does not have API for editing contact information (RAGE!), so the editing code is disabled for now. Upstream patch is already waiting for review: https://bugs.freedesktop.org/show_bug.cgi?id=59609
> 
> 
> Diffs
> -----
> 
>   KTp/CMakeLists.txt 10c17bf 
>   KTp/contact-info-dialog.h PRE-CREATION 
>   KTp/contact-info-dialog.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/108498/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/01/20/contactdialog.png
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/01/20/contactdialog-edit.png
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130127/53c09850/attachment.html>