Hello, On Tuesday 27 August 2013 19:03:56 Aleix Pol wrote: > I've started looking into splitting KUnitConversion, there's some things > I'd like to discuss: > > - There's some weird constructions > regarding KDE_PLATFORM_FEATURE_BINARY_COMPATIBLE_FEATURE_REDUCTION.
This came from the KDE Platform Mobile time. This can go away. > Especially a trade off between depending on QtNetwork or using kioclient to > download a specific file (see currency.cpp). I'd suggest to have only one > code path, and I'd say using QtNetwork can be the best, especially since > the dependency on kio is at runtime. Yep, AFAICT QtNetwork is enough for KUnitConversion. > - Currently there's a directory called kunitconversion/src/kunitconversion > that contains all the files. To me it would make more sense to have > everything within kunitconversion/src. I would have changed it if it wasn't > because there's also Solid with solid/src/solid, and it's marked as Done. > Anybody has any insight about this? Right, it's not nice, it was me doing it quick and dirty at the time when we needed to start moving KUnitConversion for dependency reasons. The right way to do it would indeed to have the lot in src, and generate local forwarding headers, like David did in sonnet. > It doesn't seem to follow [1]. This policy doesn't say this construct is forbidden. :-) > - About i18n calls, it says something like we shouldn't be using i18n() > when depending on Ki18n. What should we use then? i18nc()? Not sure I understood here... BTW, what are the dependencies for KUnitConversion currently? I wonder if we want to make it a low tier if we can. > - Also it talks about: > Document all dependencies in CMakeList between frameworks (example in kauth) > > I've looked into KAuth and I don't see what's different from other > submodules. Does it mean that target_link_libraries() calls are well made? > find_package()? I used to have the following lines in kauth: include_directories( ${kcoreaddons_SOURCE_DIR}/src/jobs # for KJob ${kcoreaddons_BINARY_DIR}/src ) Since we don't use include_directories anymore for this kind of cases it disappeared. We should do that on target_link_libraries calls now but unfortunately it hasn't been done in KAuth (should be ASAP). The idea would be to make its target_link_libraries call the following: target_link_libraries(KAuth LINK_PUBLIC Qt5::Core KCoreAddons # for KJob LINK_PRIVATE Qt5::Widgets Qt5::DBus ) The idea is to have something like that in all tier 2 and tier 3 framework so that it's easier to figure out if we can get a framework down a level of what justifies a dependency. Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel