Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-02 Thread Vishesh Handa

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/#review57102
---

Ship it!


Looks fine from a Baloo point of view. I cannot comment on the PMC parts, I 
don't know the code.


plugins/baloosearch/audiosearchresulthandler.cpp
https://git.reviewboard.kde.org/r/117915/#comment39793

Considering that you're checking if the title is empty, do you want to do 
the same for the Artist and Album?



plugins/baloosearch/searchresulthandler.cpp
https://git.reviewboard.kde.org/r/117915/#comment39794

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.


- Vishesh Handa


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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-02 Thread Shantanu Tushar


 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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-02 Thread Commit Hook

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/#review57137
---


This review has been submitted with commit 
e8e58d79829f07f3821bbf9202cbf63eb509327d by Shantanu Tushar to branch master.

- Commit Hook


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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-02 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/
---

(Updated May 2, 2014, 2:22 p.m.)


Status
--

This change has been marked as submitted.


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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-01 Thread Commit Hook

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/#review57044
---


This review has been submitted with commit 
121e51d74ff50a3644f275d5cfb571326c7a7286 by Shantanu Tushar to branch master.

- Commit Hook


On April 30, 2014, 9:01 p.m., Shantanu Tushar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117915/
 ---
 
 (Updated April 30, 2014, 9:01 p.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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-01 Thread Shantanu Tushar

---
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.)


Status
--

This change has been marked as submitted.


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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-01 Thread Shantanu Tushar


 On May 1, 2014, 6:06 a.m., Commit Hook wrote:
  This review has been submitted with commit 
  121e51d74ff50a3644f275d5cfb571326c7a7286 by Shantanu Tushar to branch 
  master.

Had pushed this by mistake when trying to fix a bug, reverted it. The review is 
still open.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/#review57044
---


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