Review Request 113723: Fix KIO to build standalone, prepare for moving into its tier
Aleix Pol Gonzalez
aleixpol at kde.org
Mon Nov 11 03:23:41 UTC 2013
> On Nov. 9, 2013, 12:47 a.m., David Faure wrote:
> > staging/kio/CMakeLists.txt, line 30
> > <http://git.reviewboard.kde.org/r/113723/diff/1/?file=212052#file212052line30>
> >
> > already listed 6 lines above
I had to add it because it's a dependency-of-a-dependency. I'll add a comment about that.
> On Nov. 9, 2013, 12:47 a.m., David Faure wrote:
> > tier3/kservice/src/CMakeLists.txt, line 91
> > <http://git.reviewboard.kde.org/r/113723/diff/1/?file=212066#file212066line91>
> >
> > Why? My grepping for KServiceOffer says that it's only used in the kservice framework.
/home/kde-devel/frameworks/kdelibs/staging/kio/src/widgets/kopenwithdialog.cpp:50:27: fatal error: kserviceoffer.h: No such file or directory
#include <kserviceoffer.h>
But it wasn't needed, I removed the include and it still works.
> On Nov. 9, 2013, 12:47 a.m., David Faure wrote:
> > staging/kio/CMakeLists.txt, line 35
> > <http://git.reviewboard.kde.org/r/113723/diff/1/?file=212052#file212052line35>
> >
> > already listed 3 lines before.
Dependency of a dependency
> On Nov. 9, 2013, 12:47 a.m., David Faure wrote:
> > staging/kio/CMakeLists.txt, line 42
> > <http://git.reviewboard.kde.org/r/113723/diff/1/?file=212052#file212052line42>
> >
> > needed?
Dependency of a dependency
> On Nov. 9, 2013, 12:47 a.m., David Faure wrote:
> > tier1/kcoreaddons/src/lib/CMakeLists.txt, line 128
> > <http://git.reviewboard.kde.org/r/113723/diff/1/?file=212060#file212060line128>
> >
> > I think we should instead treat them as separate libs, and remove the inheritance from KCompositeJobPrivate in KIO::JobPrivate. It's just an optimization. We don't want to break BIC in an existing libkio if we ever change kcoreaddons' private classes.
> > (Unless we can guarantee that they are always the same version - like Qt does, but the difference is that they don't need to install private headers for this; and they have a runtime check for version mismatches).
> >
> > All in all, I think an extra call to "new" per kio job is the simplest solution; a kio job takes much longer than that anyway.
Can we then just skip this part and make this a different patch? I can do it, but it seems unrelated to me.
- Aleix
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113723/#review43285
-----------------------------------------------------------
On Nov. 11, 2013, 3:23 a.m., Aleix Pol Gonzalez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113723/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 3:23 a.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kdelibs
>
>
> Description
> -------
>
> As you will see, this splitting was a bit harder than others:
> - KIO was using a couple of private headers from kjobwidgets, which now they will be installed.
> - The xslt_kde target was being used from KDocTools without having it exported. Now it will be properly exported.
> - Also defines all dependencies so it can be compiled independently, modularization is done as well.
>
>
> Diffs
> -----
>
> staging/kio/src/ioslaves/help/CMakeLists.txt 40637dc
> staging/kio/src/filewidgets/CMakeLists.txt 31fe8c6
> staging/kio/CMakeLists.txt 6c7297e
> cmake/modules/FindGSSAPI.cmake
> cmake/modules/CMakeLists.txt 07f7eac
> staging/kio/src/ioslaves/http/kcookiejar/CMakeLists.txt 2630f01
> staging/kio/src/ioslaves/http/tests/CMakeLists.txt 52c9f6c
> staging/kio/src/widgets/CMakeLists.txt d90386d
> staging/kio/src/widgets/kopenwithdialog.cpp cb4fc0f
> staging/kio/tests/CMakeLists.txt 6cee291
> superbuild/CMakeLists.txt 53f5952
> tier1/kcoreaddons/src/lib/CMakeLists.txt 4e6e206
> tier1/kcoreaddons/src/lib/jobs/kcompositejob_p.h 20baf7c
> tier2/kdoctools/CMakeLists.txt c2256ff
> tier2/kdoctools/KDocToolsConfig.cmake d501dc8
> tier2/kdoctools/KDocToolsConfig.cmake.in PRE-CREATION
> tier2/kdoctools/src/CMakeLists.txt 3940e98
> tier3/kded/KDEDConfig.cmake.in 32f8d56
>
> Diff: http://git.reviewboard.kde.org/r/113723/diff/
>
>
> Testing
> -------
>
> Builds, Installs, tests still pass; both modularized and monolithic kdelibs.
>
>
> Thanks,
>
> Aleix Pol Gonzalez
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131111/7762717f/attachment.html>
More information about the Kde-frameworks-devel
mailing list