<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/102384/">http://git.reviewboard.kde.org/r/102384/</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/102384/diff/1/?file=32202#file32202line57" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/upnpcollection/UpnpCollectionFactory.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace Collections {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">55</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">QDBusConnection</span></span><span class="hl"> </span><span class="n"><span class="hl">bus</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="n">QDBusConnection</span><span class="o">::</span><span class="n">sessionBus</span><span class="p">()<span class="hl">;</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">57</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k"><span class="hl">if</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="o"><span class="hl">!</span></span><span class="n"><span class="hl">cagibi0_1_0Init</span></span><span class="p"><span class="hl">(</span></span><span class="n">QDBusConnection</span><span class="o">::</span><span class="n">sessionBus</span><span class="p">()<span class="hl">)</span></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;">coding style says it should looks like this:
if( !cagibi0_1_0init( QDBusConnection::sessionBus() )
Sorry to bitch about that but it really helps to review your patch if the code is readable for someone really entrenched in the amarok coding style.</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;">Needs a run though astyle (see /HACKING) but I guess it will work.
It's a bit to time intensive for me to test fully, so I'm trusting your skill.
Also, we should consider raising the minimal KDE version for next release.</pre>
<p>- Bart</p>
<br />
<p>On August 20th, 2011, 10:58 a.m., Nikhil Shantanu Marathe 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 Amarok and Bart Cerneels.</div>
<div>By Nikhil Shantanu Marathe.</div>
<p style="color: grey;"><i>Updated Aug. 20, 2011, 10:58 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;">Cagibi 0.2.0 changed the D-BUS API including method locations and the return format of deviceDetails.
At the same time, since Amarok still has KDE 4.4 as the lowest requirement, Cagibi 0.1.0 compatibility is required.
This patch adds support for both versions</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;">Tested with one device, on both Cagibi 0.1.1 and 0.2.0</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/core-impl/collections/upnpcollection/UpnpCollectionFactory.h <span style="color: grey">(f36030e)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpCollectionFactory.cpp <span style="color: grey">(9fdaa45)</span></li>
<li>src/core-impl/collections/upnpcollection/dbuscodec.h <span style="color: grey">(158896e)</span></li>
<li>src/core-impl/collections/upnpcollection/dbuscodec.cpp <span style="color: grey">(b6842cb)</span></li>
<li>src/core-impl/collections/upnpcollection/deviceinfo.h <span style="color: grey">(00b5b7f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102384/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>