----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120276/#review66919 -----------------------------------------------------------
Hey, thanks for working on this! Couple remarks below, but pretty good otherwise dataengines/comic/cachedprovider.cpp <https://git.reviewboard.kde.org/r/120276/#comment46668> QSettings::value returns QVariant, that in turn can be converted to QUrl directly rather than converting through QString Also, why is it not using KConfig? dataengines/comic/cachedprovider.cpp <https://git.reviewboard.kde.org/r/120276/#comment46669> Same here (direct toUrl()) dataengines/comic/cachedprovider.cpp <https://git.reviewboard.kde.org/r/120276/#comment46670> And here as well dataengines/comic/comic.h <https://git.reviewboard.kde.org/r/120276/#comment46671> Remove dataengines/comic/comic.h <https://git.reviewboard.kde.org/r/120276/#comment46672> Remove if unneeded dataengines/comic/comic.h <https://git.reviewboard.kde.org/r/120276/#comment46673> Remove if unneeded dataengines/comic/comic.h <https://git.reviewboard.kde.org/r/120276/#comment46674> Remove if unneeded dataengines/comic/comic_package.cpp <https://git.reviewboard.kde.org/r/120276/#comment46675> Why remove constructing the parent class? dataengines/comic/comic_package.cpp <https://git.reviewboard.kde.org/r/120276/#comment46676> Remove if unneeded dataengines/comic/comic_package_plugin.cpp <https://git.reviewboard.kde.org/r/120276/#comment46677> Does this file still serve any purpose? Remove it if not dataengines/comic/comicprovider.h <https://git.reviewboard.kde.org/r/120276/#comment46678> Let's remove this macro, it's not just a substitute for K_PLUGIN_FACTORY. So remove and replace usage with K_PLUGIN_FACTORY dataengines/comic/comicprovider.cpp <https://git.reviewboard.kde.org/r/120276/#comment46679> Make the QUrls const & --> const QUrl &oldUrl etc (in the above slot too) dataengines/comic/comicproviderkross.h <https://git.reviewboard.kde.org/r/120276/#comment46680> Remove if unneeded dataengines/comic/comicproviderkross.h <https://git.reviewboard.kde.org/r/120276/#comment46681> Remove if unneeded dataengines/comic/comicproviderkross.cpp <https://git.reviewboard.kde.org/r/120276/#comment46682> Remove if unneeded dataengines/comic/comicproviderwrapper.cpp <https://git.reviewboard.kde.org/r/120276/#comment46683> Remove if unneeded - Martin Klapetek On Sept. 19, 2014, 6:53 a.m., Andrei Amuraritei wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120276/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2014, 6:53 a.m.) > > > Review request for Plasma, David Edmundson, Marco Martin, and Sebastian > Kügler. > > > Repository: kdeplasma-addons > > > Description > ------- > > comic DataEngine initial port to frameworks. > > > Diffs > ----- > > dataengines/CMakeLists.txt 04c7985 > dataengines/comic/CMakeLists.txt 8e382e6 > dataengines/comic/cachedprovider.h baac8a9 > dataengines/comic/cachedprovider.cpp caca25e > dataengines/comic/comic.h 8cc3969 > dataengines/comic/comic.cpp 7130f44 > dataengines/comic/comic_package.h 32be381 > dataengines/comic/comic_package.cpp 6d2ff0b > dataengines/comic/comic_package_plugin.cpp d997947 > dataengines/comic/comicprovider.h 630ee8d > dataengines/comic/comicprovider.cpp ab248a5 > dataengines/comic/comicproviderkross.h 46a9072 > dataengines/comic/comicproviderkross.cpp 9820f05 > dataengines/comic/comicproviderwrapper.h 81eee68 > dataengines/comic/comicproviderwrapper.cpp 48ced42 > > Diff: https://git.reviewboard.kde.org/r/120276/diff/ > > > Testing > ------- > > Building from source, compiles 100%, some deprecated warnings. DataEngine > shows up in plasmaengineexplorer and detects installed .comic packages. > This is the initial port, still need to review code to fix issues like > whitespaces around ( or the deprecated parts. > Thanks notmart, d_ed, sebas, bshas etc for helping. > > > Thanks, > > Andrei Amuraritei > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
