----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/#review27646 -----------------------------------------------------------
Technically correct patch, but please address the remarks below before this can be merged. src/EngineController.h <http://git.reviewboard.kde.org/r/108964/#comment20719> No, no, no, please don't put this to EngineController at all. Put the slots to MainWindow.cpp, read the config value there and just call EngineController::seekRelative() src/MainWindow.cpp <http://git.reviewboard.kde.org/r/108964/#comment20720> Please make the label actually read the config, e.g. i18nc( "%1 is duration as returned by KLocale::formatDuration()", "Seek Forward by %1", KLocale::formatDuration( AmarokConfig::seekShort() * 1000 ) ) Also please pay attention to Amarok coding style - spaces around arguments (except in SIGNAL/SLOT macros. [this applies to all added KAction calls below too] src/amarokconfig.kcfg <http://git.reviewboard.kde.org/r/108964/#comment20721> Please make the default short seek 2 seconds, 5 is too close to medium seek. src/configdialog/dialogs/PlaybackConfig.ui <http://git.reviewboard.kde.org/r/108964/#comment20722> This seems to be a left-over as we've agreed that we won't add UI for these micro-options. - Matěj Laitl On Feb. 15, 2013, noon, Anmol Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108964/ > ----------------------------------------------------------- > > (Updated Feb. 15, 2013, noon) > > > Review request for Amarok. > > > Description > ------- > > Added 3 different seek options with customizable seek durations which can be > set in the playback-config: > Control + L/R - Short seek > L/R - Normal/ Medium seek > Shift + L/R - Long seek > > > Diffs > ----- > > src/EngineController.h e9a8c26 > src/EngineController.cpp 3577acf > src/MainWindow.cpp 8f985dc > src/amarokconfig.kcfg 3ebf71d > src/configdialog/dialogs/PlaybackConfig.ui 3a79e43 > > Diff: http://git.reviewboard.kde.org/r/108964/diff/ > > > Testing > ------- > > > Thanks, > > Anmol Ahuja > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
