<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/102596/">http://git.reviewboard.kde.org/r/102596/</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/102596/diff/1/?file=35998#file35998line26" style="color: black; font-weight: bold; text-decoration: underline;">src/services/amazon/AmazonCollection.h</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; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace Collections {</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">26</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">AmazonCollection</span> <span class="o">:</span> <span class="n">public</span> <span class="n">Collections</span><span class="o">::</span><span class="n">ServiceCollection</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 class seems so thin that I wonder if it's not better use do the service without using Collection and QueryMaker. Please consider doing the population of the view yourself, so it can suit the store idea better then the simple service TreeView.</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/102596/diff/1/?file=36004#file36004line87" style="color: black; font-weight: bold; text-decoration: underline;">src/services/amazon/AmazonMeta.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; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QList< QAction * > Meta::AmazonItem::customActions()</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">87</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">actions</span><span class="p">.</span><span class="n">append</span><span class="p">(</span> <span class="n">m_addToCartAction</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;">Should not be in the {} block.</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/102596/diff/1/?file=36004#file36004line93" style="color: black; font-weight: bold; text-decoration: underline;">src/services/amazon/AmazonMeta.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; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QList< QAction * > Meta::AmazonItem::customActions()</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">93</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">Meta</span><span class="o">::</span><span class="n">AmazonItem</span><span class="o">::</span><span class="n">setAsin</span><span class="p">(</span> <span class="n">QString</span> <span class="n">asin</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 prefer to use capitals, even in function names, for abbreviations. So setASIN() here. But that is just personal preference that improves readability for me.</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/102596/diff/1/?file=36005#file36005line44" style="color: black; font-weight: bold; text-decoration: underline;">src/services/amazon/AmazonParser.h</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; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">private:</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">44</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QDomDocument</span> <span class="o">*</span><span class="n">m_responseDocument</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;">How big is the responce-document going to be? Should you not rather use QXmlStreamReader rather then keeping the entire DOM in memory and having to wait for end of fetching before parsing.</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/102596/diff/1/?file=36013#file36013line37" style="color: black; font-weight: bold; text-decoration: underline;">src/services/magnatune/MagnatuneSettingsModule.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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">37</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">kDebug</span><span class="p">(</span> <span class="mi">14310</span> <span class="p">)</span> <span class="o"><<</span> <span class="s">"Creating M<span class="hl">p3</span>tune<span class="hl">s</span> config object"</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">37</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">kDebug</span><span class="p">(</span> <span class="mi">14310</span> <span class="p">)</span> <span class="o"><<</span> <span class="s">"Creating M<span class="hl">agna</span>tune config object"</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;">You probably should commit these fixes to master separately so they don't pollute the already huge patch.</pre>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Have you given any thought to a more advanced store browsing UI? In the narrow browser column you probably can only do search results, perhaps also recommendations. People expect a better experience from a music store though.
And like I said, consider doing your own UI for the browser-column (i.e. not ServiceCollection).</pre>
<p>- Bart</p>
<br />
<p>On September 13th, 2011, 12:59 a.m., Sven Krohlas 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 Amarok.</div>
<div>By Sven Krohlas.</div>
<p style="color: grey;"><i>Updated Sept. 13, 2011, 12:59 a.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;">Totally WIP! Many, many TODOs and unfinished stuff in the code... pls ignore. ;-)
I'm currently stuck at the point that my parser created meta items do not show up in the browser... any idea why?</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/browsers/CollectionTreeItemModelBase.cpp <span style="color: grey">(119f90d)</span></li>
<li>src/core-impl/collections/support/MemoryQueryMakerInternal.h <span style="color: grey">(d3825ca)</span></li>
<li>src/core/collections/Collection.h <span style="color: grey">(02b0894)</span></li>
<li>src/core/collections/QueryMaker.h <span style="color: grey">(dfca79f)</span></li>
<li>src/services/CMakeLists.txt <span style="color: grey">(ad14fda)</span></li>
<li>src/services/ServiceCollection.h <span style="color: grey">(7a57aa6)</span></li>
<li>src/services/ServiceMetaBase.cpp <span style="color: grey">(21587d3)</span></li>
<li>src/services/amazon/Amazon.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonActions.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonActions.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonCart.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonCart.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonCollection.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonCollection.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonConfig.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonConfig.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonConfigWidget.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonMeta.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonMeta.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonParser.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonParser.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonSettingsModule.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonSettingsModule.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonStore.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/AmazonStore.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/amazon/amarok_service_amazonstore_config.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/services/magnatune/MagnatuneSettingsModule.cpp <span style="color: grey">(15cc57d)</span></li>
<li>src/widgets/SearchWidget.h <span style="color: grey">(df16556)</span></li>
<li>src/widgets/SearchWidget.cpp <span style="color: grey">(4b09285)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102596/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/102596/s/256/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/09/13/amarok_amazon_very_first_compete_parsing_400x100.png" style="border: 1px black solid;" alt="Unknown entry/entries after parsing" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>