<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="https://git.reviewboard.kde.org/r/117915/">https://git.reviewboard.kde.org/r/117915/</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 2nd, 2014, 9:07 a.m. UTC, <b>Vishesh Handa</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;">Looks fine from a Baloo point of view. I cannot comment on the PMC parts, I don't know the code.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">That is cool :)</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 2nd, 2014, 9:07 a.m. UTC, <b>Vishesh Handa</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/117915/diff/1/?file=270490#file270490line60" style="color: black; font-weight: bold; text-decoration: underline;">plugins/baloosearch/audiosearchresulthandler.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">60</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">values</span><span class="p">.</span><span class="n">insert</span><span class="p">(</span><span class="n">MediaCenter</span><span class="o">::</span><span class="n">ArtistRole</span><span class="p">,</span> <span class="n">file</span><span class="p">.</span><span class="n">property</span><span class="p">(</span><span class="n">KFileMetaData</span><span class="o">::</span><span class="n">Property</span><span class="o">::</span><span class="n">Artist</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;">Considering that you're checking if the title is empty, do you want to do the same for the Artist and Album?</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;">Usually things like empty Album and Artist are handled by the Media Library which shows then as "Unknown" so there is no need to check this here.
For the title, when we haven't fetched the File metadata, we would've put the filename (foo.mp3) as the title. This check is to make sure that we don't end up erasing foo.mp3 unless the title "The Foo Song" is available in Baloo::File.
</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 2nd, 2014, 9:07 a.m. UTC, <b>Vishesh Handa</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/117915/diff/1/?file=270496#file270496line36" style="color: black; font-weight: bold; text-decoration: underline;">plugins/baloosearch/searchresulthandler.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">36</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">while</span> <span class="p">(</span><span class="n">resultIterator</span><span class="p">.</span><span class="n">next</span><span class="p">())</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;">Please keep in mind that this is sync, and you'll be blocking. You want to put it another thread via the Runnable if you want it to be async.</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;">That is fine, the media sources are themselves running in a separate thread.</pre>
<br />
<p>- Shantanu</p>
<br />
<p>On May 1st, 2014, 6:06 a.m. UTC, Shantanu Tushar wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 and Vishesh Handa.</div>
<div>By Shantanu Tushar.</div>
<p style="color: grey;"><i>Updated May 1, 2014, 6:06 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-mediacenter
</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;">Just like the Nepomuk media source, the Baloo media source needs to provide the date/time information for media. This is used to sort the media to show the more recent media first.
For images, the date/time when the photo was actually taken is used, and the file creation date/time is used for other media.</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;">Tested with all three types of media, works fine. Unit tests for the new code to follow.</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>plugins/baloosearch/CMakeLists.txt <span style="color: grey">(1ff81fb)</span></li>
<li>plugins/baloosearch/audiosearchresulthandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/baloosearch/audiosearchresulthandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/baloosearch/baloosearchmediasource.h <span style="color: grey">(e315de4)</span></li>
<li>plugins/baloosearch/baloosearchmediasource.cpp <span style="color: grey">(7ebfa61)</span></li>
<li>plugins/baloosearch/imagesearchresulthandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/baloosearch/imagesearchresulthandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/baloosearch/searchresulthandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/baloosearch/searchresulthandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/baloosearch/videosearchresulthandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/baloosearch/videosearchresulthandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117915/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>