<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/108717/">http://git.reviewboard.kde.org/r/108717/</a>
     </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On May 30th, 2013, 2:09 p.m. UTC, <b>Matěj Laitl</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/108717/diff/11/?file=145267#file145267line40" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/nepomukcollection/NepomukCollection.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 11)

    </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; ">using namespace MemoryMeta;</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="#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="k">using</span> <span class="k">namespace</span> <span class="n">MemoryMeta</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">36</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">namespace</span> <span class="n">Collections</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">38</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">using</span> <span class="k">namespace</span> <span class="n">Collections</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="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;">Okay, personal preferences aside, I'm sure that consistency within a project has more value than preferences of individual developers.

There are about 25 .cpp files in current Amarok source code that embody all the method definitions into namespace XYZ {}. On the other hand, there are about 265 files that use using namespace XYZ; syntax for function definitions. Please improve, rather than fight, consistency.</pre>
 </blockquote>



 <p>On May 30th, 2013, 6:43 p.m. UTC, <b>Edward Hades Toroshchin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It's not about consistency. Favouring the using-directive is not a question of style or preference.</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;">So it is a question of what? What is the (technical, now that you've excluded personal one) rationale for introducing this incosistency? (yes, I still view it as an inconsistency)

I also don't like "issues" being marked are resolved here without ack of the one who opened them.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On May 30th, 2013, 2:09 p.m. UTC, <b>Matěj Laitl</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/108717/diff/11/?file=145267#file145267line93" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/nepomukcollection/NepomukCollection.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 11)

    </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; ">NepomukCollection::isWritable() const</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">90</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">87</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span></pre></td>
  </tr>

 </tbody>


 
 

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">91</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">NepomukCollection</span><span class="o">::</span><span class="n">metadataChanged</span><span class="p">(</span> <span class="n">Meta</span><span class="o">::</span><span class="n">TrackPtr</span> <span class="n"><span class="hl">track</span></span><span class="hl"> </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">88</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">NepomukCollection</span><span class="o">::</span><span class="n">metadataChanged</span><span class="p">(</span> <span class="n">Meta</span><span class="o">::</span><span class="n">TrackPtr</span> <span class="p">)</span></pre></td>
  </tr>

 </tbody>


 
 

 <tbody>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">92</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">{</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">89</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">{</span></pre></td>
  </tr>

 </tbody>


 
 

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">93</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="c1">// reflect change to ouside world:</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">90</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="c1">// </span><span class="cs">TODO</span><span class="c1"></span></pre></td>
  </tr>

 </tbody>


 
 

 <tbody>

  <tr>
    <th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">94</font></th>
    <td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="kt">bool</span> <span class="n">mapsChanged</span> <span class="o">=</span> <span class="n">MapChanger</span><span class="p">(</span> <span class="n">m_mc</span><span class="p">.</span><span class="n">data</span><span class="p">()</span> <span class="p">).</span><span class="n">trackChanged</span><span class="p">(</span> <span class="n">track</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>

  <tr>
    <th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">95</font></th>
    <td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="k">if</span><span class="p">(</span> <span class="n">mapsChanged</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>

  <tr>
    <th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">96</font></th>
    <td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="c1">// while docs say somehting different,</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>

  <tr>
    <th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">97</font></th>
    <td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="c1">// collection browser doesn't update unless we emit updated()</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>

  <tr>
    <th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">98</font></th>
    <td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">emit</span> <span class="n">updated</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>


 
 

 <tbody>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">99</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">}</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">91</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><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;">I don't think you need to be an Observer anymore, do you? Or, it depends how you want to emit updated() signal, but as the tracks already have a pointer to NepomukCollection, they can somehow trigger it explicitly.

I'm going to document where collections should emit updated() soon, it will read something like:
 a) when a set of entities change (tracks, albums, composers,...) removed or added
 b) when relationship between entities change, i.e. when tracks is assigned from one composer to another etc.</pre>
 </blockquote>



 <p>On May 30th, 2013, 6:43 p.m. UTC, <b>Edward Hades Toroshchin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Detecting changes in Nepomuk is a complicated thing, which I don't want to mix with the currently discussed patch.

I suggest we discuss the Observer and update mechanisms together with it (and after we push this patch).</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;">Okay, this can be merged without any update mechanism. In that case I suggest dropping Observer inheritance from NepomukCollection altogether to provide clean start for any future update mechanism.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On May 30th, 2013, 2:09 p.m. UTC, <b>Matěj Laitl</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/108717/diff/11/?file=145267#file145267line118" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/nepomukcollection/NepomukCollection.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 11)

    </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; ">NepomukCollection::isWritable() const</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">115</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">Nepomuk2</span><span class="o">::</span><span class="n">Resource</span> <span class="n">fileRes</span><span class="p">(</span> <span class="n">url</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">102</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">DEBUG_BLOCK</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">116</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">QString</span> <span class="n">uidUrl</span> <span class="o">=</span> <span class="n">fileRes</span><span class="p">.</span><span class="n">uri</span><span class="p">().</span><span class="n">toString</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">103</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">debug</span><span class="p">()</span> <span class="o"><<</span> <span class="n">url</span><span class="p">;</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">117</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="k">return</span> <span class="nf">trackForUidUrl</span><span class="p">(</span> <span class="n">uidUrl</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">104</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="k">return</span> <span class="n">Meta</span><span class="o">::</span><span class="n">TrackPtr</span><span class="p">();</span> <span class="c1">// </span><span class="cs">TODO</span><span class="c1"></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;">We can use review request "issues" as a TODO list.

We may have to decide whether the methods should accept the "file" urls or just the "Nepomuk" ones, but this is the general problem of trackForUrl(). We could split these into trackForFileUrl() and trackForUidUrl() in TrackProvider.

Thinking about it more, we don't hopefully need the split and file:// urls should be accepted here too (if the file is tracked by Nepomuk), CollectionManager can enforce some order whey querying TrackProviders.</pre>
 </blockquote>



 <p>On May 30th, 2013, 6:43 p.m. UTC, <b>Edward Hades Toroshchin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> We can use review request "issues" as a TODO list.

I don't like it, to be honest. How about creating a bug instead? For example "consistent usage of trackForUrl in collections"
</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 didn't mean TODO list of amarok upstream issues, but issues of this patch. Just settle for current contract of the trackForFileUrl(), i.e. "file:// urls should be accepted here too (if the file is tracked by Nepomuk)". This is an important Collection method and I'd see the patch as incomplete without it, especially if the version in master manages to implement this (in a way not suitable for this patch).</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On May 30th, 2013, 2:09 p.m. UTC, <b>Matěj Laitl</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/108717/diff/11/?file=145268#file145268line46" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/nepomukcollection/NepomukInquirer.h</a>
    <span style="font-weight: normal;">

     (Diff revision 11)

    </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; ">public slots:</pre></td>
   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">39</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="cm">/**</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">43</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">NepomukInquirer</span><span class="p">(</span> <span class="n">QString</span> <span class="n">query</span><span class="p">,</span> <span class="n">std</span><span class="o">::</span><span class="n">auto_ptr</span><span class="o"><</span><span class="n">NepomukParser</span><span class="o">></span> <span class="n">parser</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;">We prefer Qt equivalents of std classes, if they exist. But reading documentation of std::auto_ptr, it doesn't seem to make sense as a function argument, you doesn't seem to want to delete it as soon as it goes out of scope (and yeah, it may be reset by being on rhs of an assignment, but that is convoluted).</pre>
 </blockquote>



 <p>On May 30th, 2013, 6:43 p.m. UTC, <b>Edward Hades Toroshchin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">std::auto_ptr makes perfect sense as a function argument, because it both clearly shows who is responsible for the pointer deallocation and enforces it.</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;">Not for me. This isn't usual for other Amarok code and I wouldn't like to see such practice introduced. The current practice it to document pointer ownership properly in docstrings.

I'd like to see this issue reopened. (ditto about closing issues without reviewer's ack)</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On May 30th, 2013, 2:09 p.m. UTC, <b>Matěj Laitl</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/108717/diff/11/?file=145274#file145274line428" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/nepomukcollection/NepomukQueryMaker.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 11)

    </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">428</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">std</span><span class="o">::</span><span class="n">auto_ptr</span><span class="o"><</span><span class="n">NepomukParser</span><span class="o">></span> <span class="n">parser</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;">QScopedPointer please</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;">Dropped? Why? Dropping concerns I raise without any comment discourages me from (very time-consuming) reviews.</pre>
<br />




<p>- Matěj</p>


<br />
<p>On May 30th, 2013, 7:12 p.m. UTC, Edward Hades Toroshchin 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 and Vishesh Handa.</div>
<div>By Edward Hades Toroshchin.</div>


<p style="color: grey;"><i>Updated May 30, 2013, 7:12 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;">nepomuk: implement custom QueryMaker</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;">Playing tracks from Nepomuk collection, browsing, filtering, adding/removing labels, statistics sync.</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/core-impl/collections/nepomukcollection/CMakeLists.txt <span style="color: grey">(642919bd7b2188c6055308eabc07319ae48e14be)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukCache.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukCache.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukCollection.h <span style="color: grey">(5737cb5861563a8f190a29badc15d9e3ef82faf1)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukCollection.cpp <span style="color: grey">(8c35e3dda0e3dc9a23affe1d18cc69de4d24d58c)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h <span style="color: grey">(66e2473f16227f462c5a3838c71f311b6594333a)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp <span style="color: grey">(19743fe02b1d0c9dd4785d01909a4231b3a69e90)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukInquirer.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukParser.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukParser.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukQueryMaker.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukQueryMaker.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/NepomukSelectors.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukAlbum.h <span style="color: grey">(8456ddb8d952e130816d01fc312c8df3146e83d1)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukAlbum.cpp <span style="color: grey">(76c9136f9d4f5ba77cac49b242b3aa3b2d844c0c)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukArtist.h <span style="color: grey">(7cf533e760b0a06e5a2316693e13f6aea0ac3dcd)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukArtist.cpp <span style="color: grey">(34d7c8f9c32f96be0ef5f81de9d58bfedb510c2d)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukComposer.h <span style="color: grey">(837c37fa5dce5c3c10e6840f618cf72430b51b3d)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukComposer.cpp <span style="color: grey">(33ba85ae11100e8ccbb5cc94a2d7d0e2007fdbd1)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukGenre.cpp <span style="color: grey">(c34831ab4812f0241c0000eef844c2ea2397ae97)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukLabel.h <span style="color: grey">(6f2e54c3c6a0d350615cea0e335ed9f5c5a0eedf)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukLabel.cpp <span style="color: grey">(7e2a313e5eac91128d0ce211c6575ac3d8d2b1ab)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukTrack.h <span style="color: grey">(9a534dddac51fc39f9d3705f95b194f9669416ab)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukTrack.cpp <span style="color: grey">(b9beca872cd7c7edc3e6b15662d0a49f42213c6d)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukYear.h <span style="color: grey">(9eca44fbcd3aab77149d864cb6d3e5d266cc8461)</span></li>

 <li>src/core-impl/collections/nepomukcollection/meta/NepomukYear.cpp <span style="color: grey">(1d6e3b1066e13fd6adba15ae6b1d81f8ca28a5d9)</span></li>

 <li>src/statsyncing/collection/CollectionProvider.cpp <span style="color: grey">(7e3bdd83214796fc37e0aef41225baedabe3fbda)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/108717/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/02/03/nepomuk-counting-querymaker.png">nepomuk-counting-querymaker.png</a></li>

 <li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-queries.png">nepomuk-queries.png</a></li>

 <li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/02/03/nepomuk-sync.png">nepomuk-sync.png</a></li>

</ul>





  </td>
 </tr>
</table>








  </div>
 </body>
</html>