<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/112888/">http://git.reviewboard.kde.org/r/112888/</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/112888/diff/3-4/?file=191364#file191364line40" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.h</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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">40</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QUrl</span> <span class="nf">url</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 getters should all be const</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/112888/diff/3-4/?file=191364#file191364line48" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.h</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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">48</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">urlChanged</span><span class="p">(</span><span class="n">QUrl</span> <span class="n">Url</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;">arguments should begin with lowercase letters, url, name, icon, genericName (or if you want to make the distinction between the arg and the method name more clear, use newUrlm newName, etc., or something telling.</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/112888/diff/3-4/?file=191365#file191365line39" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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">38</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QMimeDatabase</span> <span class="n">db</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 is only used in the !fi.isDir() branch, move it there for more narrow scoping.</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/112888/diff/3-4/?file=191365#file191365line42" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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">41</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">emit</span> <span class="nf">urlChanged</span><span class="p">(</span><span class="n">m_url</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;">I'd prefer to move this to the other signal emissions, just to make sure we're done with everything at the same time. Semantically, we're only done setting the url once all internal data is updated.</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/112888/diff/3-4/?file=191365#file191365line44" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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="k">if</span><span class="p">(</span><span class="n">m_url</span><span class="p">.</span><span class="n">isLocalFile</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;">space after if</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/112888/diff/3-4/?file=191365#file191365line46" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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">45</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">KFileItem</span> <span class="n">fileItem</span><span class="p">(</span><span class="n">KFileItem</span><span class="o">::</span><span class="n">Unknown</span><span class="p">,</span> <span class="n">KFileItem</span><span class="o">::</span><span class="n">Unknown</span><span class="p">,</span> <span class="n">m_url</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;">const</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/112888/diff/3-4/?file=191365#file191365line47" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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">46</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">QFileInfo</span> <span class="nf">fi</span><span class="p">(</span><span class="n">m_url</span><span class="p">.</span><span class="n">toLocalFile</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;">const</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/112888/diff/3-4/?file=191365#file191365line54" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/applets/icon/plugin/icon_p.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 3 - 4)
</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">53</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span> <span class="k">if</span><span class="p">(</span><span class="n">m_name</span><span class="p">.</span><span class="n">isNull</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;">space after if</pre>
</div>
<br />
<p>- Sebastian</p>
<br />
<p>On September 23rd, 2013, 2:56 p.m. UTC, Bhushan Shah 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 Plasma, Marco Martin and Sebastian Kügler.</div>
<div>By Bhushan Shah.</div>
<p style="color: grey;"><i>Updated Sept. 23, 2013, 2:56 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;">Port Icon applet to plasma2
Some issues..
-> Can be resized to 1x1.
-> Still doesn't work for folder, and external URLs.
-> Configuration dialog</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;">1) Compiles
2) Installs
3) Loads
-> Dropping application icon from KickOff - creates icon
-> Dropping files from Dolphin, both executable and normal files - creates icon
-> Dropping folder from Dolphin - creates icon applet but neither icon nor text because folder handling is not added yet.
4) Opens URL - Clicking on the icon applet opens url with kfmclient</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>plasma/generic/applets/CMakeLists.txt <span style="color: grey">(4ace449)</span></li>
<li>plasma/generic/applets/icon/CMakeLists.txt <span style="color: grey">(1094666)</span></li>
<li>plasma/generic/applets/icon/icon.h <span style="color: grey">(45fa07c)</span></li>
<li>plasma/generic/applets/icon/icon.cpp <span style="color: grey">(8288850)</span></li>
<li>plasma/generic/applets/icon/package/contents/config/main.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/generic/applets/icon/package/contents/ui/main.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/generic/applets/icon/package/metadata.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/generic/applets/icon/plasma-applet-icon.desktop <span style="color: grey">(8275ddd)</span></li>
<li>plasma/generic/applets/icon/plugin/icon_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/generic/applets/icon/plugin/icon_p.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/generic/applets/icon/plugin/iconplugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/generic/applets/icon/plugin/iconplugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/generic/applets/icon/plugin/qmldir <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112888/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot1.png">icons in panel</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot2.png">icons as launcher</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot3.png">files</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot4.png">folders</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>