> On May 2, 2014, 9:07 a.m., Vishesh Handa wrote: > > Looks fine from a Baloo point of view. I cannot comment on the PMC parts, I > > don't know the code.
That is cool :) > On May 2, 2014, 9:07 a.m., Vishesh Handa wrote: > > plugins/baloosearch/searchresulthandler.cpp, line 36 > > <https://git.reviewboard.kde.org/r/117915/diff/1/?file=270496#file270496line36> > > > > 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. That is fine, the media sources are themselves running in a separate thread. > On May 2, 2014, 9:07 a.m., Vishesh Handa wrote: > > plugins/baloosearch/audiosearchresulthandler.cpp, line 60 > > <https://git.reviewboard.kde.org/r/117915/diff/1/?file=270490#file270490line60> > > > > Considering that you're checking if the title is empty, do you want to > > do the same for the Artist and Album? 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. - Shantanu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/#review57102 ----------------------------------------------------------- On May 1, 2014, 6:06 a.m., Shantanu Tushar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/117915/ > ----------------------------------------------------------- > > (Updated May 1, 2014, 6:06 a.m.) > > > Review request for Plasma and Vishesh Handa. > > > Repository: plasma-mediacenter > > > Description > ------- > > 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. > > > Diffs > ----- > > plugins/baloosearch/CMakeLists.txt 1ff81fb > plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION > plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION > plugins/baloosearch/baloosearchmediasource.h e315de4 > plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 > plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION > plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION > plugins/baloosearch/searchresulthandler.h PRE-CREATION > plugins/baloosearch/searchresulthandler.cpp PRE-CREATION > plugins/baloosearch/videosearchresulthandler.h PRE-CREATION > plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/117915/diff/ > > > Testing > ------- > > Tested with all three types of media, works fine. Unit tests for the new code > to follow. > > > Thanks, > > Shantanu Tushar > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
