<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/102569/">http://git.reviewboard.kde.org/r/102569/</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/102569/diff/1/?file=35770#file35770line46" style="color: black; font-weight: bold; text-decoration: underline;">player/lib/models/achievementsmodel.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; ">AchievementsModel::AchievementsModel( GluonEngine::ProjectMetaData* metaData, const QString& userName, QObject* parent)</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">45</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">AchievementsModel</span><span class="o">::</span><span class="n">AchievementsModel</span><span class="p">(</span> <span class="n">GluonEngine</span><span class="o">::</span><span class="n">ProjectMetaData</span><span class="o">*</span> <span class="n">metaData</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span><span class="o">&</span> <span class="n">userName</span><span class="p">,</span> <span class="n">QObject</span><span class="o">*</span> <span class="n">parent</span><span class="p">)</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">46</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">AchievementsModel</span><span class="o">::</span><span class="n">AchievementsModel</span><span class="p">(</span> <span class="n">GluonEngine</span><span class="o">::</span><span class="n">ProjectMetaData</span><span class="o">*</span> <span class="n">metaData</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span><span class="o">&</span> <span class="n">userName</span><span class="p">,</span> <span class="n">QObject</span><span class="o">*</span> <span class="n">parent</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;">Metadata is not documented for the constructor properly so that we need a proper pass of the API user here.
There are more ways behind the documentation to handle this:
1) assert. We did at the other place for some cases.
2) silently not return any proper icon path,
I do not like the first two ones, and I would probably go fro this third one:
3) Using a proper qDebug() error message below
I could probably add a constructor expecting a projectFilePath arguement would also be a convinience in some cases, but that is not the scope of this patch. I can do that later, if the need arises.</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/102569/diff/1/?file=35770#file35770line50" style="color: black; font-weight: bold; text-decoration: underline;">player/lib/models/achievementsmodel.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; ">AchievementsModel::AchievementsModel( GluonEngine::ProjectMetaData* metaData, const QString& userName, QObject* parent)</pre></td>
<td colspan="2"><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">50</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">metaData</span> <span class="o">=</span> <span class="n">metaData</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;">The second problem is that, the d->metaData member is actually "leaking" now since it is not removed in the destructor and up to the API user what will happen with that.
One way is that to have the metaData copy here (which will be a more straight-forward decision after the projectFilePath() based constructor) and delete it in the desctructor. That way it would not be deleted from the caller "silently".
Also, we might need to document that how it happens on the API interface documentation (ie.: this will be internally deleted at the class destruction).</pre>
</div>
<br />
<p>- Laszlo</p>
<br />
<p>On September 10th, 2011, 4:30 p.m., Felix Rohrbach 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 Gluon and Laszlo Papp.</div>
<div>By Felix Rohrbach.</div>
<p style="color: grey;"><i>Updated Sept. 10, 2011, 4:30 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;">Fix showing the achievement icons in the kdeext player.</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>player/lib/models/achievementsmodel.cpp <span style="color: grey">(7f66f46)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102569/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>