<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/105629/">http://git.reviewboard.kde.org/r/105629/</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;">Good work, I see you tests have oncovered a couple of our errors, which is great.
Can you please split the actual fixes & cleanups (to be merged first) from the test? I'll try to look on the mpris conver failures, you may have uncovered another real ploblem as it was already reported on our bugzilla that MPRIS covers are sometimes wrong. (but perhaps it was fixed since then?)</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/105629/diff/1/?file=73979#file73979line34" style="color: black; font-weight: bold; text-decoration: underline;">src/core/meta/support/MetaUtility.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; "></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">34</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">static</span> <span class="k">const</span> <span class="n">QString</span> <span class="n">XESAM_ALBUM</span> <span class="o">=</span> <span class="s">"http://freedesktop.org/standards/xesam/1.0/core#album"</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;">This IMO results in multiple copies of the strings in the final binary. (each time it is included)
The strings should be declared in .h, but defined (value assigned) in .cpp, you'll much likely need to remove the static modifier.</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/105629/diff/1/?file=73984#file73984line327" style="color: black; font-weight: bold; text-decoration: underline;">tests/core/meta/support/TestMetaUtility.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; ">TestMetaUtility::testSecToPrettyTime_data()</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">327</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="o">+</span> <span class="n">qint64</span><span class="p">(</span> <span class="mi">13</span> <span class="p">)</span> <span class="o">*</span> <span class="mi">60</span> <span class="o">+</span> <span class="mi">13</span> <span class="p">)</span> <span class="o">*</span> <span class="mi">1000</span><span class="ew"> </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;">nitpick: trailing whitespace</pre>
</div>
<br />
<p>- Matěj</p>
<br />
<p>On July 20th, 2012, 3:54 p.m., Jasneet Bhatti 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, Matěj Laitl and Sven Krohlas.</div>
<div>By Jasneet Bhatti.</div>
<p style="color: grey;"><i>Updated July 20, 2012, 3:54 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;">Test for core/meta/support/MetaUtility
THIS IS NOT YET COMPLETE.
The testMprisMapFromTrack() and testMpris20MapFromTrack() test functions are not setting the cover image properly and I'm unable to understand why. I've added a couple of qDebug() statements to show the same. I'd be grateful if someone could tell me what the problem actually is.
Everything else is working fine.</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;">Builds and runs fine. Just the image setting problem exists.</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/db/sql/SqlRegistry.h <span style="color: grey">(8d80117)</span></li>
<li>src/core/meta/support/MetaUtility.h <span style="color: grey">(c1ba442)</span></li>
<li>src/core/meta/support/MetaUtility.cpp <span style="color: grey">(133076b)</span></li>
<li>tests/core/meta/CMakeLists.txt <span style="color: grey">(3ae78c9)</span></li>
<li>tests/core/meta/support/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/core/meta/support/TestMetaUtility.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/core/meta/support/TestMetaUtility.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/data/cover/amarok.png <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105629/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>