Review Request 116744: Play All Button in Plasma Media Center

[Shantanu Tushar]

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


Thanks for the patch, here are some things you can improve in this -


browsingbackends/localfiles/localfilesabstractbackend.cpp
<https://git.reviewboard.kde.org/r/116744/#comment37210>

    Looks like you accidentally added some spaces at the end of the line, remove them.



browsingbackends/localfiles/localfilesabstractbackend.cpp
<https://git.reviewboard.kde.org/r/116744/#comment37211>

    Instead of so much effort here, in handleButtonClick() you can just count the number of existing media in the playlist and then use play(n), passing the index of the first media after adding the new items.



libs/mediacenter/playlistmodel.cpp
<https://git.reviewboard.kde.org/r/116744/#comment37212>

    Please remove all qDebug() before submitting reviews



mediaelements/mediaplayer/MusicStats.qml
<https://git.reviewboard.kde.org/r/116744/#comment37213>

    Unrelated change. Make sure that the diff for a particular review request should only contain the code related to that review.



mediaelements/mediawelcome/HomeScreenFooter.qml
<https://git.reviewboard.kde.org/r/116744/#comment37214>

    Same as above.


- Shantanu Tushar


On March 11, 2014, 10:18 p.m., Atul Dubey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116744/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 10:18 p.m.)
> 
> 
> Review request for Plasma, Akshay Ratan, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan.
> 
> 
> Bugs: 331040
>     http://bugs.kde.org/show_bug.cgi?id=331040
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> -------
> 
> Tried to fix the Play All button... Now it plays the first track of the album that is added... 
> 
> Earlier, it was just getting populated in the playlist and the sog wasn't playing...
> 
> 
> Diffs
> -----
> 
>   browsingbackends/localfiles/localfilesabstractbackend.cpp faaafa7 
>   libs/mediacenter/playlistmodel.cpp 6477047 
>   mediaelements/mediaplayer/MusicStats.qml 178a37d 
>   mediaelements/mediawelcome/HomeScreenFooter.qml d2c0eb7 
> 
> Diff: https://git.reviewboard.kde.org/r/116744/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Atul Dubey
> 
>

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