----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100686/#review1518 -----------------------------------------------------------
Almost all new code doesn't follow amarok code formating style (check out http://quickgit.kde.org/?p=amarok.git&a=blob&f=HACKING/intro_and_style.txt this doc) + odd initial and trailing white spaces. New dialog looks cut (http://img810.imageshack.us/i/filenameconfigdialog.png/). A lot of free spaces on a right side and shrunken fields on the left. src/core-impl/podcasts/sql/PodcastFilenameLayoutConfigDialog.cpp <http://git.reviewboard.kde.org/r/100686/#comment1281> What is Apply button here for? src/core-impl/podcasts/sql/PodcastFilenameLayoutConfigDialog.cpp <http://git.reviewboard.kde.org/r/100686/#comment1280> Better to use QLatin1String here, and why don't write just filenameLayout == QLatin1String( "%default" ) instead? src/core-impl/podcasts/sql/SqlPodcastProvider.cpp <http://git.reviewboard.kde.org/r/100686/#comment1279> We have lovely QStringx class exactly for such purposes (#include "QStringx.h"), It also supports optional fields (inside braces {}). But It uses %name% scheme (as OrganizeCollection and TagGuesser does). - Sergey On Feb. 19, 2011, 5:36 p.m., Sandeep Raghuraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100686/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2011, 5:36 p.m.) > > > Review request for Amarok and Bart Cerneels. > > > Summary > ------- > > Adds a dialog to configure podcast episode filenames. Added a filenamelayout > column to podcastchannels table and a member to PodcastMeta to hold the > filename layout. Podcast database version updated to 5. > > > This addresses bug 155075. > https://bugs.kde.org/show_bug.cgi?id=155075 > > > Diffs > ----- > > src/CMakeLists.txt d47daf4 > src/core-impl/podcasts/sql/PodcastFilenameLayoutConfigDialog.h PRE-CREATION > src/core-impl/podcasts/sql/PodcastFilenameLayoutConfigDialog.cpp > PRE-CREATION > src/core-impl/podcasts/sql/PodcastFilenameLayoutConfigWidget.ui > PRE-CREATION > src/core-impl/podcasts/sql/PodcastSettingsBase.ui e1f36e7 > src/core-impl/podcasts/sql/PodcastSettingsDialog.h 08fdad3 > src/core-impl/podcasts/sql/PodcastSettingsDialog.cpp de53d77 > src/core-impl/podcasts/sql/SqlPodcastMeta.cpp 777825e > src/core-impl/podcasts/sql/SqlPodcastProvider.cpp 9784d08 > src/core/podcasts/PodcastMeta.h e11b2eb > src/core/podcasts/PodcastMeta.cpp fb82fa8 > > Diff: http://git.reviewboard.kde.org/r/100686/diff > > > Testing > ------- > > > Thanks, > > Sandeep > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel