----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100088/#review301 -----------------------------------------------------------
shared/FileType.cpp <http://git.reviewboard.kde.org/r/100088/#comment246> What's a point to have "Other" as supported/possible file types? Tbh, I don't know where It could be used. I can be wrong. But If remove this, you were not need KLocale, and you can change ( a bit ) enum with types, to make It like Unknown = -1, Mp3 = 0, ..., so you can just return Amarok::FileType( list.indexOf( extension.toLower() ) ); shared/FileType.cpp <http://git.reviewboard.kde.org/r/100088/#comment245> It can be started from 1, since no one needs "Other" extension. :) And what relates to entire patch. As Leo noticed, you should use amarok code formating style. And It would be better to set const modifier to func args since you don't going to modify 'em, and use links instead of copying whole variable in case of strings. - Sergey On 2010-11-05 10:46:54, Stefan Derkits wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100088/ > ----------------------------------------------------------- > > (Updated 2010-11-05 10:46:54) > > > Review request for Amarok. > > > Summary > ------- > > -) Created new Class with some Helper Functions > -) Changed MetaQueryWidget to use the QStringList that contains all Filetypes > -) Meta::SqlTrack.type() now gets it's Information from the Database instead > of the FileExtension > > > Diffs > ----- > > src/browsers/CollectionTreeItemModelBase.cpp 3f64a73 > shared/FileType.cpp PRE-CREATION > shared/FileType.h 55c80b9 > src/core-impl/collections/support/MemoryFilter.cpp e2509ac > src/widgets/MetaQueryWidget.cpp 0249edf > tests/synchronization/CMakeLists.txt e5df2df > > Diff: http://git.reviewboard.kde.org/r/100088/diff > > > Testing > ------- > > Tested in the App, found no wrong Behaviour > > > Thanks, > > Stefan > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
