Review Request 117365: Add Genre to metadata PMC keeps

[Shantanu Tushar]

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


Works fine, I can see the values in the table. However, the actual set of the value should happen inside Media instead of MediaLibrary as the comments below.


libs/mediacenter/media.h
<https://git.reviewboard.kde.org/r/117365/#comment38362>

    doesnt need to be const QString for the return, just QString will do.
    The const QString& you see at other places is quite some bad luck and needs to be fixed to QString as well, separately.
    For more http://techbase.kde.org/Policies/Library_Code_Policy#Const_References



libs/mediacenter/medialibrary.cpp
<https://git.reviewboard.kde.org/r/117365/#comment38363>

    Instead of handling this here, you should let media->setValueForRole take care of updating the value. Just implement MediaCenter::GenreRole in Media::setValueForRole.
    
    Also, if you set wasUpdated to true always, the point of wasUpdated is lost. It is supposed to be set to true only when the old value and new value are different (which is when the setValueForRole method is supposed to return true).
    
    At the end, this else if should be removed (notice how there is no else if for createdAt (extractAndSaveDurationInfo should not have been there as well, it was missed and should be removed separately).



libs/mediacenter/medialibrary.cpp
<https://git.reviewboard.kde.org/r/117365/#comment38364>

    same as above



plugins/kdedesktopsearch/kdemetadatamediasource.cpp
<https://git.reviewboard.kde.org/r/117365/#comment38365>

    Just for consistency, move this to the line after CreatedAtRole


- Shantanu Tushar


On April 4, 2014, 10:51 a.m., Ashish Madeti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117365/
> -----------------------------------------------------------
> 
> (Updated April 4, 2014, 10:51 a.m.)
> 
> 
> Review request for Plasma, Shantanu Tushar and Sinny Kumari.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> -------
> 
> Added genre to metadata pmc keeps so that it too can be retrieved from the media library when needed.
> 
> 
> Diffs
> -----
> 
>   libs/mediacenter/media.h 2bdef4d 
>   libs/mediacenter/media.cpp 32b9f19 
>   libs/mediacenter/mediacenter.h a2855dd 
>   libs/mediacenter/mediacenter.cpp 35240d3 
>   libs/mediacenter/medialibrary.cpp 713827a 
>   libs/mediacenter/pmcmedia.h 68275f2 
>   libs/mediacenter/pmcmedia.cpp a266a26 
>   libs/mediacenter/pmcmetadatamodel.cpp 2d5fd6b 
>   plugins/kdedesktopsearch/kdemetadatamediasource.cpp e8f18eb 
> 
> Diff: https://git.reviewboard.kde.org/r/117365/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashish Madeti
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140404/13dd2455/attachment-0001.html>