----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110520/#review32835 -----------------------------------------------------------
Looks good. I agree with Mark that we would want to handle plugin unloading too, but I recognise it would be much more tricky (and physical unlinking of the dynamically linked-in shared library may be close to impossible) and can be done in future, it certainly doesn't have to be in this patch. Below are some suggestions to deduplicate the code a bit a make it more self-testing. src/core-impl/collections/support/CollectionManager.cpp <http://git.reviewboard.kde.org/r/110520/#comment24369> I think the CollectionManager::init() method should loose the factories parameter, PluginManager should be adapted to call handleNewFactories() even during the normal startup. This way the code-patch would be shared, which is IMO always good as the frequent use-case "tests" the rarely used one. src/core-impl/collections/support/CollectionManager.cpp <http://git.reviewboard.kde.org/r/110520/#comment24370> We've started an effort to make amarok --debug more meningful even when not filtered. One requirement is that only very important and seldom called methods should have unconditional DEBUG_BLOCK. This one doesn't seem to be needed. - Matěj Laitl On May 19, 2013, 8:43 a.m., Edward Hades Toroshchin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110520/ > ----------------------------------------------------------- > > (Updated May 19, 2013, 8:43 a.m.) > > > Review request for Amarok. > > > Description > ------- > > create new collections without restart > > > Diffs > ----- > > src/PluginManager.cpp fc825cece9d06a92a9a84a1dcfa949bfc6c14ed8 > src/core-impl/collections/support/CollectionManager.h > a3cb6cefc958fb2875c7bef075f4a5319fc8b958 > src/core-impl/collections/support/CollectionManager.cpp > 0775206655b47d660c92bf6ebf23623de356bb69 > > Diff: http://git.reviewboard.kde.org/r/110520/diff/ > > > Testing > ------- > > > Thanks, > > Edward Hades Toroshchin > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
