<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113222/">http://git.reviewboard.kde.org/r/113222/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 13th, 2013, 5:34 a.m. UTC, <b>Lasath Fernando</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hey,
This is a nice clean patch, and it's great that someone got around to handling delivery messages in MessagesModel.
However, I don't understand why you bothered creating a MessagePrivate class instead of just adding new attributes to KTp::Message. There's no real reason not to, since the reason we created KTp::Message was so we can arbitrarily add properties like this.
I think you'd be much better off doing it that way, and the code would be cleaner :-)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Actually I said not to do that.
It's really only the message model that should be altering these things, therefore the data structure should be private to it.
If it was external the model would need some way to know the message has changed - which would mean making Message inherit from QObject to emit signals and it all becomes rather messy.</pre>
<br />
<p>- David</p>
<br />
<p>On October 12th, 2013, 8:26 p.m. UTC, Leon Handreke wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy.</div>
<div>By Leon Handreke.</div>
<p style="color: grey;"><i>Updated Oct. 12, 2013, 8:26 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ktp-common-internals
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Only tested receiving a Tp::DeliveryStatusDelivered. Not sure how to test the other cases.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>KTp/Declarative/messages-model.cpp <span style="color: grey">(9cf1606555edc7ed9f36970a957a854caf3010a0)</span></li>
<li>KTp/Declarative/messages-model.h <span style="color: grey">(a0f15653c3644f26dc768fe7ac882b4ac3c91367)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113222/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>