Review Request 120276: Initial port to frameworks for the comic dataengine.

[Martin Klapetek]

> On Oct. 13, 2014, 2:02 p.m., Martin Klapetek wrote:
> > dataengines/comic/comic_package_plugin.cpp, line 21
> > <https://git.reviewboard.kde.org/r/120276/diff/3/?file=317968#file317968line21>
> >
> >     I think this should just go into comic_package.cpp to follow all the other exports, then this file can be removed
> 
> Andrei Amuraritei wrote:
>     This is exported in comicproviderkross.cpp because otherwise the engine doesn't detect the .comic packages. I get a plugin "garfield" could be created message when testing the engine with plasmaengineexplorer.

This has nothing to do with with comicproviderkross.cpp, it should go to comic_package.cpp. Btw. this file is not even being built (it's nowhere in CMakeLists.txt), I guess it's added to the build system only because of the included moc file. Just move this one line at the bottom of comic_package.cpp.


> On Oct. 13, 2014, 2:02 p.m., Martin Klapetek wrote:
> > dataengines/comic/CMakeLists.txt, lines 51-61
> > <https://git.reviewboard.kde.org/r/120276/diff/3/?file=317961#file317961line51>
> >
> >     These two could be merged into one
> 
> Andrei Amuraritei wrote:
>     Don't know if LINK_INTERFACE_LIBRARIES is really needed here so for commented out. Need some input from someone more knowledgeable about cmake building.

My point was that you should be able to just write

target_link_libraries( plasmacomicprovidercore
    KF5::WidgetsAddons
    KF5::KIOCore
    KF5::KrossCore
    KF5::KrossUi
    KF5::I18n
    KF5::KDELibs4Support
 LINK_INTERFACE_LIBRARIES
    KF5::KDELibs4Support
)

..instead of two separate calls. Also, if you commented it out and it still works, then it's probably useless here.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120276/#review68304
-----------------------------------------------------------


On Oct. 17, 2014, 2:08 a.m., Andrei Amuraritei wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120276/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2014, 2:08 a.m.)
> 
> 
> Review request for Plasma, David Edmundson, Marco Martin, Martin Klapetek, 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.
> 
> Update: Engine is working...still need to port away from Solid and KService to remove KDELibs4Support, that is still wip. 
> 
> 
> Thanks,
> 
> Andrei Amuraritei
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20141017/9124ee94/attachment.html>