Review Request 109464: Patch which tries to fix the ordering error while tracks are copied from the hard disk to some other storage device

Matěj Laitl matej at laitl.cz
Wed Mar 27 12:56:33 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109464/#review29950
-----------------------------------------------------------


Unfortunately, the patch in its current form is of no value to Amarok. The intent was obviously NOT to turn every (in many cases valid) use of QSet to QList. :-(


src/browsers/CollectionTreeView.h
<http://git.reviewboard.kde.org/r/109464/#comment22311>

    Contains lot of changes unrelated to the goal, some of which are obviously wrong (looks like shooting blindly to me)



src/browsers/CollectionTreeView.h
<http://git.reviewboard.kde.org/r/109464/#comment22312>

    I don't think this is really needed and that this will help in any way.



src/browsers/CollectionTreeView.cpp
<http://git.reviewboard.kde.org/r/109464/#comment22313>

    Unrelated change.



src/browsers/CollectionTreeView.cpp
<http://git.reviewboard.kde.org/r/109464/#comment22314>

    Introduces style errors.



src/context/applets/lyrics/LyricsApplet.cpp
<http://git.reviewboard.kde.org/r/109464/#comment22315>

    Unrelated change that changes things to worse.



src/context/applets/lyrics/LyricsApplet.cpp
<http://git.reviewboard.kde.org/r/109464/#comment22316>

    We obviously don't want extra line between "else if"s



src/context/applets/lyrics/LyricsApplet.cpp
<http://git.reviewboard.kde.org/r/109464/#comment22317>

    Unrelated changes, whitespace errors. Please learn how to work wish git and your editor first!


- Matěj Laitl


On March 14, 2013, 7:43 p.m., mayank jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109464/
> -----------------------------------------------------------
> 
> (Updated March 14, 2013, 7:43 p.m.)
> 
> 
> Review request for Amarok, Anmol Ahuja, Matěj Laitl, and Myriam Schweingruber.
> 
> 
> Description
> -------
> 
> This patch maintains the order of artists upto the call of getKIOCopyableUrls() function
> 
> 
> This addresses bug 254404.
>     https://bugs.kde.org/show_bug.cgi?id=254404
> 
> 
> Diffs
> -----
> 
>   src/browsers/CollectionTreeView.h 3b2ca80 
>   src/browsers/CollectionTreeView.cpp aac232a 
>   src/context/applets/lyrics/LyricsApplet.cpp 2394964 
>   src/context/engines/lyrics/LyricsEngine.h b187b73 
>   src/context/engines/lyrics/LyricsEngine.cpp 2befa91 
> 
> Diff: http://git.reviewboard.kde.org/r/109464/diff/
> 
> 
> Testing
> -------
> 
> seems to be working!
> 
> 
> Thanks,
> 
> mayank jha
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130327/98e4da98/attachment-0001.html>


More information about the Amarok-devel mailing list