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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to