Review Request: Baby steps for Chat Plasmoid...

Lasath Fernando kde at lasath.org
Tue Jan 10 10:00:47 UTC 2012



> On Jan. 9, 2012, 1:54 p.m., David Edmundson wrote:
> > lib/conversation.cpp, line 30
> > <http://git.reviewboard.kde.org/r/103629/diff/2/?file=45687#file45687line30>
> >
> >     model is a poor choice of variable name. It could be a model of anything

Yeah, I've been meaning to rename it (as you can see by the comment above).
I just keep putting this off because it'll mean going through almost ALL of the QML code.


> On Jan. 9, 2012, 1:54 p.m., David Edmundson wrote:
> > lib/conversation-target.h, line 52
> > <http://git.reviewboard.kde.org/r/103629/diff/4/?file=46254#file46254line52>
> >
> >     Do we still want/need this?

to be honest, I'm not sure anymore.


> On Jan. 9, 2012, 1:54 p.m., David Edmundson wrote:
> > lib/conversation.cpp, line 30
> > <http://git.reviewboard.kde.org/r/103629/diff/4/?file=46257#file46257line30>
> >
> >     Adjust to QObject* target as discussed on IRC

I think it was just ConversationTarget that we needed to do that for, as MessagesModel has a default constructor.


> On Jan. 9, 2012, 1:54 p.m., David Edmundson wrote:
> > lib/conversation-target.h, line 31
> > <http://git.reviewboard.kde.org/r/103629/diff/4/?file=46254#file46254line31>
> >
> >     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)

I took a quick look through and I'm not sure how. What I needed this class to do was take a Tp::ContactPtr and spew out anything that could be useful from it.

If I'm to use ChannelContactModel, either I'd have to add new properties, which would destroy the point of it being a model. Or I have to add extra roles, but then the only way to notify they've changed is with the dataChanged signal which would update all roles for the contact, which I think would be pretty wasteful.


- Lasath


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


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/20120110/91706a12/attachment-0001.html>


More information about the KDE-Telepathy mailing list