Review Request 113278: GSoC 2013 - Advanced Importers - 4/4: Other changes in the repository

[Matěj Laitl]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20131114/33a31ff4/attachment.html>