----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118864/#review63073 -----------------------------------------------------------
There are a few style errors, otherwise the intent looks good. I'd have to think more about any possible side-effect before giving it final "Ship it", though. src/core/collections/CollectionLocation.cpp <https://git.reviewboard.kde.org/r/118864/#comment43807> You are using tabs to indent, which is forbidden by Amarok coding style. (please see the HACKING folder) src/core/collections/CollectionLocation.cpp <https://git.reviewboard.kde.org/r/118864/#comment43803> excess whitespace, please remove - Matěj Laitl On July 24, 2014, 4:19 p.m., Robert Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118864/ > ----------------------------------------------------------- > > (Updated July 24, 2014, 4:19 p.m.) > > > Review request for Amarok. > > > Repository: amarok > > > Description > ------- > > If the user hasn't turned off the dialog which notifies if they want > directories deleting they get prompted even if the directory is not empty. > amarok ought to only prompt when it needs the information. Maybe I should > select the don't prompt in future option but I prefer to know what's going on! > > > Diffs > ----- > > src/core/collections/CollectionLocation.cpp 209d6b4 > > Diff: https://git.reviewboard.kde.org/r/118864/diff/ > > > Testing > ------- > > Built > Tested deleting tracks on empty directories - prompted > Tested directories containing music and just containing other files - not > prompted > > Do I need a bug/enhancement report to tie to this? > > I wondered whether this change might be inefficient if the collection has > many levels but seems to be ok at least with my testing. > > > Thanks, > > Robert Marshall > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
