<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/116525/">https://git.reviewboard.kde.org/r/116525/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 28th, 2014, 3:35 p.m. CCT, <b>Thibaut Gridel</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="https://git.reviewboard.kde.org/r/116525/diff/5/?file=255728#file255728line43" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/marble/RemoteIconLoader.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<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">43</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QUrl</span> <span class="n">m_url</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;">The class is used through a static, and those attributes sound like they should be parameters passing in the methods. Otherwise the code would not be reentrant.
Try adding parameters in eg searchLocalDir and initiateDownload and those attributes should become useless.</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for mentioning it. Member m_url is useless if we pass the url in function parameters. :)</p></pre>
<br />
<p>- Abhinav</p>
<br />
<p>On March 21st, 2014, 9:02 a.m. CCT, Abhinav Gangwar wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Marble, Bernhard Beschow and Thibaut Gridel.</div>
<div>By Abhinav Gangwar.</div>
<p style="color: grey;"><i>Updated March 21, 2014, 9:02 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=310464">310464</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
marble
</div>
<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;">The patch adds support to download remote images</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;">Works fine on my system</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/lib/marble/CMakeLists.txt <span style="color: grey">(bb6f312)</span></li>
<li>src/lib/marble/PlacemarkLayout.cpp <span style="color: grey">(408607b)</span></li>
<li>src/lib/marble/RemoteIconLoader.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/marble/RemoteIconLoader.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/marble/VisiblePlacemark.h <span style="color: grey">(879e384)</span></li>
<li>src/lib/marble/VisiblePlacemark.cpp <span style="color: grey">(8811598)</span></li>
<li>src/lib/marble/geodata/data/GeoDataIconStyle.h <span style="color: grey">(9a4abbf)</span></li>
<li>src/lib/marble/geodata/data/GeoDataIconStyle.cpp <span style="color: grey">(b8310cf)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116525/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>