<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/100141/">http://git.reviewboard.kde.org/r/100141/</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;">Mostly gcc is the best reviewer for this sort of stuff. So the "it compiles? ... ship it!" philosophy works well. anyways after you incorporate the suggestions of mine that you want, just go ahead and merge.</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/100141/diff/1/?file=3426#file3426line228" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/playdarcollection/PlaydarCollection.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">228</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QPointer</span><span class="o"><</span> <span class="n">Playdar</span><span class="o">::</span><span class="n">ProxyResolver</span> <span class="o">></span> <span class="n">proxyResolver</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">228</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q<span class="hl">Weak</span>Pointer</span><span class="o"><</span> <span class="n">Playdar</span><span class="o">::</span><span class="n">ProxyResolver</span> <span class="o">></span> <span class="n">proxyResolver</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;">for a pointer that immediately goes out of scope, there really isn't much point in using qweakpointer (or qpointer)</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/100141/diff/1/?file=3428#file3428line124" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/playdarcollection/PlaydarMeta.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; ">Meta::PlaydarTrack::sid() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">124</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">m_collection</span><span class="p">.</span><span class="n"><span class="hl">isNull</span></span><span class="p">()</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">124</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">m_collection</span><span class="p">.</span><span class="n"><span class="hl">data</span></span><span class="p">()</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;">why not just "return (m_collection.data())"</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/100141/diff/1/?file=3428#file3428line290" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/playdarcollection/PlaydarMeta.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; ">Meta::PlaydarTrack::finishedPlaying( double playedFraction )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">288</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">m_collection</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">288</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">m_collection</span><span class="p"><span class="hl">.</span></span><span class="n"><span class="hl">data</span></span><span class="p"><span class="hl">()</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;">same as above</pre>
</div>
<br />
<p>- Ian</p>
<br />
<p>On November 8th, 2010, 7:58 p.m., Andy Coder wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/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 Mark Kretschmann.</div>
<div>By Andy Coder.</div>
<p style="color: grey;"><i>Updated 2010-11-08 19:58:42</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;">Drops QPointer in favor of QWeakPointer in src/core-impl/collections/playdarcollection, as was done for other code in [0] and recommended in [1].
Nothing works differently, but since it is, in number of changes at least, non-trivial, (and I've been fairly absent for a while), I figured I ought to put it up for review. In particular, anyone more familiar with QWeakPointer checking to see if I did something stupid that might cause hard to find problems down the road would be appreciated.
[0] - http://git.reviewboard.kde.org/r/100011/
[1] - http://git.reviewboard.kde.org/r/100001/</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;">Compiled, ran, played around. Worked fine for me.</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/playdarcollection/PlaydarCollection.h <span style="color: grey">(31aba11)</span></li>
<li>src/core-impl/collections/playdarcollection/PlaydarCollection.cpp <span style="color: grey">(845d3fd)</span></li>
<li>src/core-impl/collections/playdarcollection/PlaydarMeta.h <span style="color: grey">(2599b7c)</span></li>
<li>src/core-impl/collections/playdarcollection/PlaydarMeta.cpp <span style="color: grey">(755191a)</span></li>
<li>src/core-impl/collections/playdarcollection/PlaydarQueryMaker.h <span style="color: grey">(277a582)</span></li>
<li>src/core-impl/collections/playdarcollection/PlaydarQueryMaker.cpp <span style="color: grey">(590c0d8)</span></li>
<li>src/core-impl/collections/playdarcollection/support/Controller.h <span style="color: grey">(1db079a)</span></li>
<li>src/core-impl/collections/playdarcollection/support/Query.h <span style="color: grey">(01971f7)</span></li>
<li>src/core-impl/collections/playdarcollection/support/Query.cpp <span style="color: grey">(d586047)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100141/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>