----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113278/#review43694 -----------------------------------------------------------
Ship it! Green light from me. Please see some minor remarks below. src/configdialog/dialogs/MetadataConfig.cpp <http://git.reviewboard.kde.org/r/113278/#comment31397> Just to know: new enough Qt Designer supports setting icons by name - fill in the icon name into icon->Theme field so that it is filled into the .ui file. src/configdialog/dialogs/MetadataConfig.cpp <http://git.reviewboard.kde.org/r/113278/#comment31398> I'd prefer having this split into 2 lines as this may look like = vs. == bug on the first sight. src/configdialog/dialogs/MetadataConfig.cpp <http://git.reviewboard.kde.org/r/113278/#comment31399> ditto a bit too condensed src/configdialog/dialogs/MetadataConfig.ui <http://git.reviewboard.kde.org/r/113278/#comment31395> I think that KDE Human Interface Guidelines say that buttons that open dialogs should have "..." appended to the label. (but please rather check it) src/configdialog/dialogs/MetadataConfig.ui <http://git.reviewboard.kde.org/r/113278/#comment31396> ditto "..." - Matěj Laitl On Nov. 6, 2013, 8:36 p.m., Konrad Zemek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113278/ > ----------------------------------------------------------- > > (Updated Nov. 6, 2013, 8:36 p.m.) > > > Review request for Amarok. > > > Repository: amarok > > > Description > ------- > > This review contains other changes made as a part of my project, i.e. changes > outside of src/statsyncing , src/importers and tests/importers directories. > > > Diffs > ----- > > images/icons/hi16-action-view-importers-banshee-amarok.png PRE-CREATION > images/icons/hi16-action-view-importers-clementine-amarok.png PRE-CREATION > images/icons/hi16-action-view-importers-rhythmbox-amarok.png PRE-CREATION > images/icons/hi22-action-view-importers-banshee-amarok.png PRE-CREATION > images/icons/hi22-action-view-importers-clementine-amarok.png PRE-CREATION > images/icons/hi22-action-view-importers-rhythmbox-amarok.png PRE-CREATION > images/icons/hi32-action-view-importers-banshee-amarok.png PRE-CREATION > images/icons/hi32-action-view-importers-clementine-amarok.png PRE-CREATION > images/icons/hi32-action-view-importers-rhythmbox-amarok.png PRE-CREATION > images/icons/hi48-action-view-importers-banshee-amarok.png PRE-CREATION > images/icons/hi48-action-view-importers-clementine-amarok.png PRE-CREATION > images/icons/hi48-action-view-importers-rhythmbox-amarok.png PRE-CREATION > src/CMakeLists.txt 70fb67b > src/PluginManager.cpp c48a99b > src/configdialog/dialogs/MetadataConfig.h 72ef69b > src/configdialog/dialogs/MetadataConfig.cpp 7d19f93 > src/configdialog/dialogs/MetadataConfig.ui ff5d332 > src/configdialog/dialogs/PluginsConfig.cpp d5011b8 > src/core-impl/collections/support/CollectionManager.h 062418c > src/core/support/PluginFactory.h 8acd354 > src/databaseimporter/DatabaseImporter.h 602f332 > src/databaseimporter/DatabaseImporter.cpp 00dd895 > src/databaseimporter/SqlBatchImporter.h PRE-CREATION > src/databaseimporter/SqlBatchImporter.cpp PRE-CREATION > src/databaseimporter/SqlBatchImporterConfig.h PRE-CREATION > src/databaseimporter/SqlBatchImporterConfig.cpp PRE-CREATION > src/databaseimporter/amarok14/FastForwardImporter.h 76a2519 > src/databaseimporter/amarok14/FastForwardImporter.cpp 31e1c3c > src/databaseimporter/amarok14/FastForwardImporterConfig.h 21048c7 > src/databaseimporter/amarok14/FastForwardImporterConfig.cpp e803cf5 > src/databaseimporter/amarok14/FastForwardWorker.h 6a4640f > src/databaseimporter/amarok14/FastForwardWorker.cpp b40b8a3 > src/databaseimporter/itunes/ITunesImporter.h a8b8f54 > src/databaseimporter/itunes/ITunesImporter.cpp 5da190e > src/databaseimporter/itunes/ITunesImporterConfig.h 78fe10b > src/databaseimporter/itunes/ITunesImporterConfig.cpp 8aa9733 > src/databaseimporter/itunes/ITunesImporterWorker.h e8a104d > src/databaseimporter/itunes/ITunesImporterWorker.cpp e7149c8 > src/databaseimporter/sqlbatch/SqlBatchImporter.h f233652 > src/databaseimporter/sqlbatch/SqlBatchImporter.cpp 79bcd0c > src/databaseimporter/sqlbatch/SqlBatchImporterConfig.h fab735c > src/databaseimporter/sqlbatch/SqlBatchImporterConfig.cpp 26b68d4 > src/dialogs/CollectionSetup.cpp 2b120e3 > src/dialogs/DatabaseImporterDialog.h f0ae7c3 > src/dialogs/DatabaseImporterDialog.cpp 5159ce8 > src/services/lastfm/SynchronizationTrack.h d6fb593 > src/services/lastfm/SynchronizationTrack.cpp 39c51d4 > tests/CMakeLists.txt e18aaa1 > tests/CollectionTestImpl.h 183c3c2 > tests/mocks/MetaMock.h 37a50ce > > Diff: http://git.reviewboard.kde.org/r/113278/diff/ > > > Testing > ------- > > > Thanks, > > Konrad Zemek > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
