----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106511/#review19574 -----------------------------------------------------------
Please finish the cleanups and other refactorings - the patch as it is looks like first part of something bigger that makes it a bit unclean. src/EqualizerController.h <http://git.reviewboard.kde.org/r/106511/#comment15562> Please remove the singleton pattern. src/EqualizerController.h <http://git.reviewboard.kde.org/r/106511/#comment15561> Whitespace on blank lines! We all hate the red colour! ;) src/EqualizerController.h <http://git.reviewboard.kde.org/r/106511/#comment15563> Please remove the singleton pattern. src/EqualizerController.cpp <http://git.reviewboard.kde.org/r/106511/#comment15564> First own include, then Amarok includes using #include "path/to/Something.h" style, then KDE includes, then Qt includes, then everything else, groups separated by one blank line. - Matěj Laitl On Sept. 21, 2012, 12:45 a.m., Ryan McCoskrie wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106511/ > ----------------------------------------------------------- > > (Updated Sept. 21, 2012, 12:45 a.m.) > > > Review request for Amarok. > > > Description > ------- > > This patch moves much of the Equalizer Dialogues internal behaviour into a > dedicated object accessed via The::equalizer()*. Any component of Amarok > looking to adjust equalizer levels (i.e. The equalizer scripting support I'll > resubmit some day) should use this interface. > > I'm considering as my next steps, merging the EqualizerPresets class with the > EqualizerController class and possibly removing dependence on AmarokConfig to > make the actual changes.. > > *The equalizer dialogue is accessed via The::equalizerDialog() > > > Diffs > ----- > > src/CMakeLists.txt 8596144 > src/EqualizerController.h PRE-CREATION > src/EqualizerController.cpp PRE-CREATION > src/MainWindow.cpp 0530fd7 > src/core/support/Components.h c38977c > src/core/support/Components.cpp 22e5de0 > src/dialogs/EqualizerDialog.h 9dad055 > src/dialogs/EqualizerDialog.cpp 3a63fe6 > > Diff: http://git.reviewboard.kde.org/r/106511/diff/ > > > Testing > ------- > > Checked that Amarok compiles and that the equalizer dialogue still works. > Found that enabling/dis-enabling the equalizer forces the track to freeze or > restart. Pressing stop and then play will make it continue with the correct > preset enabled. > > > Thanks, > > Ryan McCoskrie > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
