<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/102156/">http://git.reviewboard.kde.org/r/102156/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy and Martin Klapetek.</div>
<div>By George Goldberg.</div>
<p style="color: grey;"><i>Updated July 31, 2011, 6:29 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">After discussions on IRC with d_ed, I did a bit of qualitative testing and I reckon that he's right about the loops in dataChanged handling being crap. So, I replaced them with signals proxied via the Item class as suggested. This takes away all the frequently-used loops in PersonSetModel.
Also quickly implemented the personsCount/onlinePersonsCount for groups that was missing in the previous patch.</pre>
</td>
</tr>
</table>
<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;">This patch to telepathy-kde takes the GroupsModel from the contactlist and refactors it into a subclass of PersonSetModel.
Martin: I'm not entirely convinced I understood your algorithm in GroupsModel for when the groups of a Person change, so I might have ended up making it considerably more inefficient in this review request. Please shout if you see anything that should be changed.
While writing this summary, I've just realised that I forgot to implement the [Online]UsersCount in this, so assume that I'm aware of that and will add it in a later version of the patch :)</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;">PersonSetModel still passes modeltest after the refactoring, and the contactlist behaves as expected when adjusted to use this model instead of the GroupsModel directly.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/CMakeLists.txt <span style="color: grey">(b0cf53b6bcf6f2b6395ea24f79f50d6d5344d6fd)</span></li>
<li>src/KTelepathy/GroupedPersonSetModel <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/contact.h <span style="color: grey">(64bc4de4508f3ccce97e248b979bbc30f23452e2)</span></li>
<li>src/core/contact.cpp <span style="color: grey">(246f2afad0ac377aba9e4b94fac5d5d7faa148bf)</span></li>
<li>src/core/person.cpp <span style="color: grey">(a690a18b926282cb3368cb077cad6371619db61e)</span></li>
<li>src/ui/grouped-person-set-model.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/ui/grouped-person-set-model.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/ui/grouped-person-set-model_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/ui/person-set-model.h <span style="color: grey">(38860ae456a83d698ed19868fbabf4fed6555902)</span></li>
<li>src/ui/person-set-model.cpp <span style="color: grey">(6d01c29f7e7173e2b8af99a709fc3a2137d3115d)</span></li>
<li>src/ui/person-set-model_p.h <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102156/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>