<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 31st, 2011, 2:50 p.m., <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/102156/diff/1/?file=30460#file30460line60" style="color: black; font-weight: bold; text-decoration: underline;">src/ui/grouped-person-set-model.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void GroupedPersonSetModelPrivate::setRolesForQML()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">60</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">q</span><span class="o">-></span><span class="n">setRoleNames</span><span class="p">(</span><span class="n">roles</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think this needs to append the role names from the PersonSetModel too.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">PersonSetModelPrivate::setRolesFOrQML() on line 53 should do that.</pre>
<br />
<p>- George</p>
<br />
<p>On July 31st, 2011, 11:37 a.m., George Goldberg wrote:</p>
<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, 11:37 a.m.</i></p>
<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> </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/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>