<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/106228/">http://git.reviewboard.kde.org/r/106228/</a>
</td>
</tr>
</table>
<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/106228/diff/1/?file=81369#file81369line91" style="color: black; font-weight: bold; text-decoration: underline;">TelepathyLoggerQt4/log-manager.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; ">bool LogManager::clearHistory()</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">91</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QDBusInterface</span> <span class="n">iface</span><span class="p">(</span><span class="n">QLatin1String</span><span class="p">(</span><span class="s">"org.freedesktop.Telepathy.Logger"</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;">don't do this.
1) Sync calls are bad
2) Tp has a special way of generating Dbus bindings, where you provide an XML file and it magically builds everything. This already exists in the TpLoggerQt. If you open (in your build folder) _gen/cli-logger.h you will find it actually has these calls.
They should still be wrapped in the LogManager, just like you've done, but instead we should keep an instance of the auto generated LoggerInterface and use that. Also calls should be wrapped in Tp::PendingOperation
so we get something like
clearAccountHistory(account)
{
return Tp::PendingVoid(m_interface->ClearAccount(account->objectPath()), account);
}
For any other project this would be absolutely fine, but TpQt has their own special way, so we should follow that.
I should have mentioned it in the bug, but I didn't think about it till now.</pre>
</div>
<br />
<p>- David</p>
<br />
<p>On August 26th, 2012, 11:10 p.m., Dan Vratil 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 David Edmundson.</div>
<div>By Dan Vratil.</div>
<p style="color: grey;"><i>Updated Aug. 26, 2012, 11:10 p.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 three new methods to Tpl::LogManager to invoke telepathy-logger DBus methods for clearing logs.</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;">Works, be careful :)</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>TelepathyLoggerQt4/log-manager.h <span style="color: grey">(c245965)</span></li>
<li>TelepathyLoggerQt4/log-manager.cpp <span style="color: grey">(e6acf40)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106228/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>