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

[Andrei Amuraritei]

> On Oct. 13, 2014, 3:02 p.m., Martin Klapetek wrote:
> > dataengines/comic/CMakeLists.txt, line 12
> > <https://git.reviewboard.kde.org/r/120276/diff/3/?file=317961#file317961line12>
> >
> >     Is the KDELibs4Support needed only because of the KStandardDirs? If yes, then let's port away from that as well, it's easy enough

KDELibs4Support still needed for some other stuff (Solid and KService), need to port away from those, still wrapping my head around them.


> On Oct. 13, 2014, 3: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

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.


> On Oct. 13, 2014, 3: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

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.


> On Oct. 13, 2014, 3:02 p.m., Martin Klapetek wrote:
> > dataengines/comic/comicproviderwrapper.cpp, lines 762-771
> > <https://git.reviewboard.kde.org/r/120276/diff/3/?file=317974#file317974line762>
> >
> >     The coding style is no spaces inside ()s (I know it's not your code, but since you're touching it already, let's fix it)
> >     
> >     Also, do we need all this kind of information actually printed in the log?

For now I think it's better to keep it. I am working on porting the applet also and until we have a working version I'd like to keep it here.


- Andrei


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


On Oct. 17, 2014, 2:59 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:59 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
> 
>

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