<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/113390/">http://git.reviewboard.kde.org/r/113390/</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/113390/diff/1/?file=204537#file204537line484" style="color: black; font-weight: bold; text-decoration: underline;">src/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">484</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c">#shouldn't be here</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;">So where should it be? :)</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/113390/diff/1/?file=204540#file204540line596" style="color: black; font-weight: bold; text-decoration: underline;">src/amarokconfig.kcfg</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">596</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <entry key="EnableDeprecationWarnings" type="Bool"></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 user probably does not want to see meaningless errors about some script they installed. This is a debugging feature for developers, right?</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/113390/diff/1/?file=204552#file204552line255" style="color: black; font-weight: bold; text-decoration: underline;">src/configdialog/dialogs/ScriptsConfig.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">244</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KMessageBox</span><span class="o">::</span><span class="n">error</span><span class="p">(</span> <span class="k">this</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"Please select a script!"</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;">Don't use exclamation marks in user-visible strings.
Also I'm wondering why an error dialog is shown here. Maybe the button should simply be disabled if nothing is selected. Then there would be no need for error handling.</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/113390/diff/1/?file=204552#file204552line260" style="color: black; font-weight: bold; text-decoration: underline;">src/configdialog/dialogs/ScriptsConfig.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">249</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">response</span> <span class="o">=</span> <span class="n">KMessageBox</span><span class="o">::</span><span class="n">warningContinueCancel</span><span class="p">(</span> <span class="k">this</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"You are advised to only uninstall manually "</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;">Please reword the string. It's pretty unclear.
E.g. "Default scripts that are bundled with Amarok cannot be uninstalled."</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/113390/diff/1/?file=204552#file204552line289" style="color: black; font-weight: bold; text-decoration: underline;">src/configdialog/dialogs/ScriptsConfig.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">278</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">KArchiveFile</span> <span class="o">*</span><span class="n">file</span> <span class="o">=</span> <span class="n">findSpecFile</span><span class="p">(</span> <span class="n">subDir</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;">Can be changed to:
return findSpecFile( subdir );
</pre>
</div>
<br />
<p>- Mark Kretschmann</p>
<br />
<p>On October 22nd, 2013, 4:06 p.m. UTC, Anmol Ahuja 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 Amarok.</div>
<div>By Anmol Ahuja.</div>
<p style="color: grey;"><i>Updated Oct. 22, 2013, 4:06 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
amarok
</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;">Miscellaneous changes made in the src/ folder.</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/App.h <span style="color: grey">(9ee071d)</span></li>
<li>src/App.cpp <span style="color: grey">(eb3f483)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(70fb67b)</span></li>
<li>src/EngineController.h <span style="color: grey">(80ec17b)</span></li>
<li>src/EngineController.cpp <span style="color: grey">(cf29cf8)</span></li>
<li>src/amarokconfig.kcfg <span style="color: grey">(84cfa5b)</span></li>
<li>src/amarokurls/BookmarkGroup.h <span style="color: grey">(ea42c30)</span></li>
<li>src/browsers/CollectionTreeItem.h <span style="color: grey">(0a5d197)</span></li>
<li>src/browsers/CollectionTreeItem.cpp <span style="color: grey">(4c35523)</span></li>
<li>src/browsers/CollectionTreeItemModelBase.h <span style="color: grey">(00e99fb)</span></li>
<li>src/browsers/CollectionTreeView.h <span style="color: grey">(454a70d)</span></li>
<li>src/browsers/CollectionTreeView.cpp <span style="color: grey">(6fad164)</span></li>
<li>src/browsers/collectionbrowser/CollectionWidget.h <span style="color: grey">(7480015)</span></li>
<li>src/browsers/collectionbrowser/CollectionWidget.cpp <span style="color: grey">(95a16dc)</span></li>
<li>src/configdialog/dialogs/ScriptSelector.h <span style="color: grey">(9ce1168)</span></li>
<li>src/configdialog/dialogs/ScriptSelector.cpp <span style="color: grey">(117cedf)</span></li>
<li>src/configdialog/dialogs/ScriptsConfig.h <span style="color: grey">(6a47847)</span></li>
<li>src/configdialog/dialogs/ScriptsConfig.cpp <span style="color: grey">(714b63e)</span></li>
<li>src/configdialog/dialogs/ScriptsConfig.ui <span style="color: grey">(b4b8d37)</span></li>
<li>src/context/applets/lyrics/LyricsApplet.cpp <span style="color: grey">(7db356d)</span></li>
<li>src/context/applets/songkick/SongkickApplet.cpp <span style="color: grey">(ed93e56)</span></li>
<li>src/context/engines/lyrics/LyricsEngine.cpp <span style="color: grey">(d3273b0)</span></li>
<li>src/core-impl/collections/support/jobs/WriteTagsJob.h <span style="color: grey">(a92ab8d)</span></li>
<li>src/core-impl/collections/support/jobs/WriteTagsJob.cpp <span style="color: grey">(5834cd4)</span></li>
<li>src/core/collections/Collection.h <span style="color: grey">(94d24d7)</span></li>
<li>src/core/collections/QueryMaker.h <span style="color: grey">(92b6a65)</span></li>
<li>src/core/meta/support/MetaConstants.cpp <span style="color: grey">(40d7122)</span></li>
<li>src/dialogs/DiagnosticDialog.cpp <span style="color: grey">(c9bfce6)</span></li>
<li>src/dialogs/EqualizerDialog.h <span style="color: grey">(6c9e19e)</span></li>
<li>src/dialogs/EqualizerDialog.cpp <span style="color: grey">(acf0da0)</span></li>
<li>src/dialogs/deviceconfiguredialog.cpp <span style="color: grey">(e54edad)</span></li>
<li>src/dynamic/BiasFactory.cpp <span style="color: grey">(2a887c2)</span></li>
<li>src/dynamic/TrackSet.h <span style="color: grey">(dee8ee3)</span></li>
<li>src/dynamic/TrackSet.cpp <span style="color: grey">(456a10a)</span></li>
<li>src/equalizer/EqualizerPresets.h <span style="color: grey">(be8d267)</span></li>
<li>src/equalizer/EqualizerPresets.cpp <span style="color: grey">(b9aa13b)</span></li>
<li>src/playback/EqualizerController.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/playback/EqualizerController.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/playlistmanager/PlaylistManager.h <span style="color: grey">(a835c35)</span></li>
<li>src/services/scriptable/ScriptableService.cpp <span style="color: grey">(f254e3e)</span></li>
<li>src/services/scriptable/ScriptableServiceInfoParser.cpp <span style="color: grey">(998135b)</span></li>
<li>src/services/scriptable/ScriptableServiceManager.cpp <span style="color: grey">(aa28a6c)</span></li>
<li>src/services/scriptable/ScriptableServiceQueryMaker.cpp <span style="color: grey">(07233dd)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113390/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>