----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109752/#review29956 -----------------------------------------------------------
Looks rather good, please resolve minor issues below. Also please add ChangeLog entry under CHANGES (on top) mentioning the bug number. WRT: to testing: what have you tested with, udisks1 or udisks2? Which KDE version? What happens with mixed audio CD with both CDDA tracks and data track? src/core-impl/collections/umscollection/UmsCollection.cpp <http://git.reviewboard.kde.org/r/109752/#comment22328> I think this check should be moved up above the while loop, as the StoregeAccess device should be directly the OpticalDisc device. src/core-impl/collections/umscollection/UmsCollection.cpp <http://git.reviewboard.kde.org/r/109752/#comment22325> nitpick: Amarok conding style says "OpticalDisc *opt" rather than "OpticalDisc * opt". Also "opt" sounds like "option" to me, "disc" would be a better variable name. src/core-impl/collections/umscollection/UmsCollection.cpp <http://git.reviewboard.kde.org/r/109752/#comment22326> "if (" -> "if(" src/core-impl/collections/umscollection/UmsCollection.cpp <http://git.reviewboard.kde.org/r/109752/#comment22327> This debugging statement is only useful when developing please remove it. (along with the braces) - Matěj Laitl On March 26, 2013, 10:57 p.m., Anmol Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109752/ > ----------------------------------------------------------- > > (Updated March 26, 2013, 10:57 p.m.) > > > Review request for Amarok. > > > Description > ------- > > Added a check for data CDs in UmsCollection.cpp so they can be handled by > UmsCollection in Amarok > > > Diffs > ----- > > src/core-impl/collections/umscollection/UmsCollection.cpp 2b5c53b > > Diff: http://git.reviewboard.kde.org/r/109752/diff/ > > > Testing > ------- > > Tested a data CD to see if it plays > > > Thanks, > > Anmol Ahuja > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
