> On Sept. 17, 2013, 1:30 a.m., David Faure wrote: > > knewstuff/CMakeLists.txt, line 9 > > <http://git.reviewboard.kde.org/r/112730/diff/1/?file=189541#file189541line9> > > > > Does this really build when building all of kdelibs in one go? I > > thought we determined that find_package of kf5 components can't work as > > long as we build everything together? E.g. I see no > > find_package(KWindowSystem) in tier2/knotifications. > > > > If you want it to build both ways I think you have to add if (NOT > > kdelibs_SOURCE_DIR) around the find_package(K*).
In looking at other libraries such as http://git.reviewboard.kde.org/r/112811/ it seems if (NOT kdelibs_SOURCE_DIR) isn't needed. why would find_package(KWindowSystem) need to be in teir2/knotifications ? > On Sept. 17, 2013, 1:30 a.m., David Faure wrote: > > knewstuff/src/CMakeLists.txt, line 72 > > <http://git.reviewboard.kde.org/r/112730/diff/1/?file=189542#file189542line72> > > > > This can be improved: > > > > use LINK_PUBLIC for libs whose classes appear in the knewstuff API, and > > LINK_PRIVATE for libs which are purely an implementation detail of > > knewstuff. > > > > In addition, add a comment on each line about why the dependency is > > required, like > > ${KArchive_LIBRARIES} # for KZip > > > > This helps spotting unnecessary deps, or spotting what could be done to > > reduce deps. Ok, I'll do that for the next diff I upload > On Sept. 17, 2013, 1:30 a.m., David Faure wrote: > > knewstuff/src/downloaddialog.cpp, line 35 > > <http://git.reviewboard.kde.org/r/112730/diff/1/?file=189543#file189543line35> > > > > to answer your question about why this is necessary, I typed "make > > downloaddialog.cpp.i" in my build (i.e. without this patch applied). This > > shows me the preprocessed file, which gets #include <klocalizedstring.h> > > from ui_downloadwidget.h. > > > > This is added there by staging/kde4support/src/kde4uic.cmake > > > > So we have a problem. When not using kde4uic, we end up with tr() > > instead of i18n() (more precisely tr2i18n()) in uic-generated files. > > This is a problem for libs and apps which use ki18n, since I think they > > need to use either one of the translation frameworks, but not mix them. > > Right, Chusslove? Albert? Anyone with a clue? :-) > > > > Should we reintroduce kde4uic.cmake in the ki18n framework (with a > > better name), to have this work like in kde4? > > > > Yep, doing so in a different review. - Jeremy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112730/#review40193 ----------------------------------------------------------- On Sept. 17, 2013, 1:29 a.m., Jeremy Whiting wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112730/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2013, 1:29 a.m.) > > > Review request for KDE Frameworks, Albert Astals Cid, David Faure, and > Chusslove Illich. > > > Description > ------- > > This makes it so I can mkdir build; cd build; cmake ../; make install from > knewstuff sources. > It's still using KDE4_KIO_LIBS and find_package(KIO) since not all of the kio > libraries have been split out apparently. > I'm not sure why sources had to be changed, but I had to add includes of > klocalizedstring where we didn't need them before somehow. > > > Diffs > ----- > > knewstuff/CMakeLists.txt 3fccbc6ee01a1cc8920321ee7125efb0bfc68412 > knewstuff/src/CMakeLists.txt c31398159459b79160ef76f193d6208d19953b4d > knewstuff/src/downloaddialog.cpp 3294c7c04c7879320fc0949db0310868bd6fa4fa > knewstuff/src/downloadwidget.cpp 64b7673d67b4e2f15007fc1a3f57d3da844d1dc0 > knewstuff/src/ui/entrydetailsdialog.cpp > 65b75d79941d9026f368f82c7b6df91d754e0925 > knewstuff/src/uploaddialog.cpp dbde573e8c3a477755c8c866d0ca1fccd1a35729 > > Diff: http://git.reviewboard.kde.org/r/112730/diff/ > > > Testing > ------- > > It builds and installs. > > > Thanks, > > Jeremy Whiting > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel