Review Request 113222: Handle message delivery reports in MessagesModel

Leon Handreke leonh at ndreke.de
Sun Oct 13 22:13:17 UTC 2013



> On Oct. 13, 2013, 10:34 a.m., David Edmundson wrote:
> > KTp/Declarative/messages-model.cpp, line 161
> > <http://git.reviewboard.kde.org/r/113222/diff/1/?file=200900#file200900line161>
> >
> >     We're missing a crucial step.
> >     
> >     You update the data in the mode, but the view won't update.
> >     
> >     Every time you change the data in the model you have to call
> >     
> >     dataChanged(index,index);
> >     where index is the data you've changed so the views update.
> >     
> >     
> >

I assume you wanted to say "emit" instead of "call" (or does it have the same effect?). I added a Q_EMIT.


> On Oct. 13, 2013, 10:34 a.m., David Edmundson wrote:
> > KTp/Declarative/messages-model.cpp, line 152
> > <http://git.reviewboard.kde.org/r/113222/diff/1/?file=200900#file200900line152>
> >
> >     watch your indentation.
> >     
> >     (though this could be just reviewboard showing it wrong)

It looks fine to me, even in RB, but I might have different ideas of how the indentation should be. The if is indented 4 spaces deeper than the for block, and the continuation of the for line aligns with the opening parentheses of the for. Should it be different?

Anyway, this line of code is gone now.


- Leon


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


On Oct. 12, 2013, 8:26 p.m., Leon Handreke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113222/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2013, 8:26 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> -------
> 
> Handle message delivery reports and update original message object with the the new status and possibly an error message.
> 
> Introduce a new internal MessagePrivate class to hold extra properties for MessagesModel to avoid having to extend KTp::Message.
> 
> 
> Diffs
> -----
> 
>   KTp/Declarative/messages-model.cpp 9cf1606555edc7ed9f36970a957a854caf3010a0 
>   KTp/Declarative/messages-model.h a0f15653c3644f26dc768fe7ac882b4ac3c91367 
> 
> Diff: http://git.reviewboard.kde.org/r/113222/diff/
> 
> 
> Testing
> -------
> 
> Only tested receiving a Tp::DeliveryStatusDelivered. Not sure how to test the other cases.
> 
> 
> Thanks,
> 
> Leon Handreke
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20131013/461d1a58/attachment.html>


More information about the KDE-Telepathy mailing list