<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/109524/">http://git.reviewboard.kde.org/r/109524/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 16th, 2013, 7:22 p.m. UTC, <b>Stephen Kelly</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/109524/diff/1/?file=120104#file120104line2148" style="color: black; font-weight: bold; text-decoration: underline;">tier1/itemmodels/src/kselectionproxymodel.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; ">QModelIndex KSelectionProxyModel::mapToSource(const QModelIndex &proxyIndex) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2148</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">proxyIndex</span><span class="p">.</span><span class="n">internalPointer</span><span class="p">()</span> <span class="o">>=</span> <span class="mi">0</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Why do you remove this?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This assert checks if a pointer is not negative.
Why we want check that?</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 16th, 2013, 7:22 p.m. UTC, <b>Stephen Kelly</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/109524/diff/1/?file=120105#file120105line135" style="color: black; font-weight: bold; text-decoration: underline;">tier1/kcodecs/autotests/kcharsetstest.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; ">void KCharsetsTest::testFromEntity()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">135</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="hl"> </span><span class="n"><span class="hl">KCharsets</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="n"><span class="hl">singleton</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span> <span class="n">KCharsets</span><span class="o">::</span><span class="n">charsets</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">135</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KCharsets</span><span class="o">::</span><span class="n">charsets</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Warnings are sometimes valid and create a todo list. Changes like this look like just hiding the warning for the sake of it. It looks like 'done' for this is something different, not just removing the variable.
Please double check that the other warnings you are resolving should really be resolved in a naive way.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think that this variable is a copy&paste from the other tests of this class. In this test we also have a FIX comment. Why don't remove the warning?
</pre>
<br />
<p>- Miquel</p>
<br />
<p>On March 16th, 2013, 6:16 p.m. UTC, Miquel Canes Gonzalez wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and Aleix Pol Gonzalez.</div>
<div>By Miquel Canes Gonzalez.</div>
<p style="color: grey;"><i>Updated March 16, 2013, 6:16 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;">Remove some compile warnings like unused variables, reordering initializers, uint -> int...</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;">compile again with less warnings.</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>interfaces/kimproxy/library/kimproxy.cpp <span style="color: grey">(66e263f)</span></li>
<li>kdecore/io/kdebug.cpp <span style="color: grey">(6fd7584)</span></li>
<li>kdeui/dialogs/kaboutapplicationpersonmodel_p.cpp <span style="color: grey">(717a6cd)</span></li>
<li>kdeui/dialogs/kedittoolbar.cpp <span style="color: grey">(401a292)</span></li>
<li>kdeui/dialogs/kinputdialog.cpp <span style="color: grey">(28ce074)</span></li>
<li>kdeui/dialogs/kshortcutseditordelegate.cpp <span style="color: grey">(d458106)</span></li>
<li>kdeui/kernel/kstyle.cpp <span style="color: grey">(681b940)</span></li>
<li>kdeui/tests/kreplacetest.h <span style="color: grey">(72674eb)</span></li>
<li>kdeui/tests/kreplacetest.cpp <span style="color: grey">(c59bd76)</span></li>
<li>kdeui/util/kmodifierkeyinfoprovider_dummy.cpp <span style="color: grey">(d456e82)</span></li>
<li>kdeui/widgets/kdatetable.cpp <span style="color: grey">(4d24290)</span></li>
<li>kdeui/widgets/kkeysequencewidget.cpp <span style="color: grey">(28eb47e)</span></li>
<li>kio/bookmarks/kbookmarkimporter_opera.cc <span style="color: grey">(d39f7a4)</span></li>
<li>kio/kio/ksambashare.cpp <span style="color: grey">(0239af5)</span></li>
<li>kio/tests/kbookmarktest.cpp <span style="color: grey">(3a0c742)</span></li>
<li>kioslave/file/file.cpp <span style="color: grey">(d57517b)</span></li>
<li>knewstuff/knewstuff3/ui/itemsgridviewdelegate.cpp <span style="color: grey">(eb72240)</span></li>
<li>staging/ki18n/src/klocalizedstring.cpp <span style="color: grey">(452dd59)</span></li>
<li>staging/kwidgets/src/icons/kiconengine.cpp <span style="color: grey">(3e5fcc0)</span></li>
<li>tier1/itemmodels/src/kdescendantsproxymodel.cpp <span style="color: grey">(8ee0702)</span></li>
<li>tier1/itemmodels/src/kselectionproxymodel.cpp <span style="color: grey">(f002f44)</span></li>
<li>tier1/kcodecs/autotests/kcharsetstest.cpp <span style="color: grey">(0c7aac1)</span></li>
<li>tier1/kjs/src/kjs/array_object.cpp <span style="color: grey">(b726c09)</span></li>
<li>tier1/kjs/src/kjs/bytecode/machine.cpp.in <span style="color: grey">(a434c36)</span></li>
<li>tier1/kwindowsystem/src/kxutils.cpp <span style="color: grey">(f3e276a)</span></li>
<li>tier1/solid/src/solid/backends/upnp/upnpinternetgateway.cpp <span style="color: grey">(2c4ca85)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109524/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>