<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/109403/">http://git.reviewboard.kde.org/r/109403/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The code looks good, just a couple of comments...
</pre>
<br />
<div>
<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/109403/diff/1/?file=119078#file119078line13" style="color: black; font-weight: bold; text-decoration: underline;">CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">13</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span> <span class="p">(</span><span class="s">KTP_MESSAGE_FILTER_FRAMEWORK_VERSION</span> <span class="s2">"<span class="hl">3</span>"</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">13</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span> <span class="p">(</span><span class="s">KTP_MESSAGE_FILTER_FRAMEWORK_VERSION</span> <span class="s2">"<span class="hl">4</span>"</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You will need to change KTP_MESSAGE_FILTER_FRAMEWORK_VERSION in all the plugins, or they won't work anymore.</pre>
</div>
<br />
<div>
<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/109403/diff/1/?file=119080#file119080line52" style="color: black; font-weight: bold; text-decoration: underline;">KTp/abstract-message-filter.h</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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class KTP_EXPORT AbstractMessageFilter : public QObject</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">50</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">void</span> <span class="nf">filterOutgoingMessage</span><span class="p">(</span><span class="n">KTp</span><span class="o">::</span><span class="n">Message</span> <span class="o">&</span><span class="n">message</span><span class="p">,</span> <span class="k">const</span> <span class="n">KTp</span><span class="o">::</span><span class="n">MessageContext</span> <span class="o">&</span><span class="n">context</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">52</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">void</span> <span class="nf">filterOutgoingMessage</span><span class="p">(</span><span class="n">KTp</span><span class="o">::</span><span class="n"><span class="hl">PendingSend</span>Message</span> <span class="o">&</span><span class="n">message</span><span class="p">,</span> <span class="k">const</span> <span class="n">KTp</span><span class="o">::</span><span class="n">MessageContext</span> <span class="o">&</span><span class="n">context</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is there a reason to leave the context in this method?</pre>
</div>
<br />
<div>
<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/109403/diff/1/?file=119084#file119084line40" style="color: black; font-weight: bold; text-decoration: underline;">KTp/pending-send-message.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">40</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">KTP_EXPORT</span> <span class="n">PendingSendMessage</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">PendingSendMessage doesn't look like a good name to me, it make me think about "Pending" stuff in TpQt that have a completely different meaning. Perhaps OutgoingMessage, or PlainTextMessage?</pre>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Also before a "ship it!" I'd like to see the other patch porting all the plugins to the new version :)</pre>
<p>- Daniele E.</p>
<br />
<p>On March 11th, 2013, 1:05 a.m. UTC, David Edmundson 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 David Edmundson.</div>
<p style="color: grey;"><i>Updated March 11, 2013, 1:05 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;">Add a new class for processing outgoing messages
Currently we re-use KTp::Message when the filters process a message to be sent, this exposes a lot of setters and getters that have no use at all.
It also mixes HTML and plain text which has already led to one bug, and would probably lead to more.
This keeps the API cleaner and simpler
--
This is an API and ABI break in the middle of a feature freeze, which is normally against the results.
I want this patch in because I want the API to be sensible for people to start writing their own plugins, I don't want them to break them for 0.7</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>CMakeLists.txt <span style="color: grey">(323afb3c965f28eeea84f51956b7aa55a7b5588e)</span></li>
<li>KTp/CMakeLists.txt <span style="color: grey">(448425593b6b1c2f0191c03e54871b7f7bc87ff5)</span></li>
<li>KTp/abstract-message-filter.h <span style="color: grey">(cf26907c5849b4df4ad725b2a5f7cb9f73953866)</span></li>
<li>KTp/abstract-message-filter.cpp <span style="color: grey">(06d805db1e9e440ae95cdd0368c8ba3212c2bc4e)</span></li>
<li>KTp/message-processor.h <span style="color: grey">(a22fc707a22f9171728d859387d21eff99afdbe2)</span></li>
<li>KTp/message-processor.cpp <span style="color: grey">(2e252f9e81b622c5673f0037cd127f3381a337b4)</span></li>
<li>KTp/pending-send-message.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>KTp/pending-send-message.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109403/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>