Review Request: Baby steps for Chat Plasmoid...

[David Edmundson]

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



lib/chat-window-style.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7876>

    



lib/conversation.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7878>

    model is a poor choice of variable name. It could be a model of anything



lib/conversation.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7877>

    Don't do this. Pass proper parents to "model" and target.



lib/conversation-target.h
<http://git.reviewboard.kde.org/r/103629/#comment7949>

    FUTURE: I think we can possibly merge what this does with the channel-contact-model class. Discuss.
    
    (ChannelContactModel being a model of all people currently in a channel. If you're in a 1-1 chat, we could just bind to the first contact. Maybe) 



lib/conversation-target.h
<http://git.reviewboard.kde.org/r/103629/#comment7944>

    get rid of default constructor (as discussed on IRC)
    
    
    have a parameter for QObject* parent too.
    
    (pass this to the QObject constructor in the class).



lib/conversation-target.h
<http://git.reviewboard.kde.org/r/103629/#comment7945>

    Do we still want/need this?



lib/conversation-target.h
<http://git.reviewboard.kde.org/r/103629/#comment7946>

    if you get rid of setContact, get rid of this too.



lib/conversation.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7943>

    Adjust to QObject* target as discussed on IRC



lib/conversation.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7940>

    const &   const &



lib/conversation.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7948>

    Pass "this" as parents then you don't have to worry about deleting them yourself. 



lib/conversation.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7941>

    Don't do this, pass "this" as a parent like other QObjects



lib/conversation.cpp
<http://git.reviewboard.kde.org/r/103629/#comment7942>

    as above.
    
    (also never call "delete" on a QObject in generate use myObject->deleteLater(); 
    
    without getting technical, it's generally safer.
    
    Though passing parents properly is safest yet :-)



lib/conversations-model.h
<http://git.reviewboard.kde.org/r/103629/#comment7947>

    add QObject *parent.


- David Edmundson


On Jan. 9, 2012, 11:52 a.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103629/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2012, 11:52 a.m.)
> 
> 
> Review request for Telepathy and David Edmundson.
> 
> 
> Description
> -------
> 
> Right, I decided that I should have reviewed and/or merge what work I've done so far rather than letting it pile up in its corner into a giant worthless clump. 
> 
> To the people that haven't heard (or wasn't at woshibon), this is a chat plasmoid that more or less behaves like facebook and google talk, except it sits in your taskbar :)
> 
> And in terms of feedback, at this stage I think design issues should take priority over sane code because the main reason I'm doing this is because I don't want to have to do any massive restructuring later on.
> 
> And if things don't make sense, ask me (I didn't comment/document anything well and I'm certainly too sleepy now ;)
> 
> PS: All the code is in my scratch repo
> http://quickgit.kde.org/?p=clones%2Ftelepathy-text-ui%2Ffernando%2Fqmlplugin.git&a=shortlog&h=refs/heads/qml_plugins2
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt d1cc185 
>   lib/CMakeLists.txt 5d39a62 
>   lib/conversation-que-manager.h PRE-CREATION 
>   lib/conversation-que-manager.cpp PRE-CREATION 
>   lib/conversation-target.h PRE-CREATION 
>   lib/conversation-target.cpp PRE-CREATION 
>   lib/conversation.h PRE-CREATION 
>   lib/conversation.cpp PRE-CREATION 
>   lib/conversations-model.h PRE-CREATION 
>   lib/conversations-model.cpp PRE-CREATION 
>   lib/messages-model.h PRE-CREATION 
>   lib/messages-model.cpp PRE-CREATION 
>   lib/qml-plugins.h PRE-CREATION 
>   lib/qml-plugins.cpp PRE-CREATION 
>   lib/qmldir PRE-CREATION 
>   lib/telepathy-text-observer.h PRE-CREATION 
>   lib/telepathy-text-observer.cpp PRE-CREATION 
>   plasmoid/CMakeLists.txt PRE-CREATION 
>   plasmoid/contents/ui/ChatWidget.qml PRE-CREATION 
>   plasmoid/contents/ui/ConversationDelegate.qml PRE-CREATION 
>   plasmoid/contents/ui/TextDelegate.qml PRE-CREATION 
>   plasmoid/contents/ui/main.qml PRE-CREATION 
>   plasmoid/metadata.desktop PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/103629/diff/diff
> 
> 
> Testing
> -------
> 
> Um.. yeah... about that...    :/
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120109/4b2bbad5/attachment-0001.html>