Re: Review Request 113723: Fix KIO to build standalone, prepare for moving into its tier
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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113723: Fix KIO to build standalone, prepare for moving into its tier
On Nov. 11, 2013, 9:54 a.m., David Faure wrote: tier1/kcoreaddons/src/lib/jobs/kcompositejob_p.h, line 30 http://git.reviewboard.kde.org/r/113723/diff/2/?file=212461#file212461line30 Err why did you remove the inheritance from KJobPrivate, which is in the same library as KCompositeJobPrivate?? If this is from my advice, it's a misunderstanding. My advice was to cut the inheritance between kio and kcoreaddons. Not within kcoreaddons. Sorry, I didn't mean to send this change. - Aleix --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113723/#review43411 --- 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113723: Fix KIO to build standalone, prepare for moving into its tier
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113723/ --- (Updated Nov. 11, 2013, 11:40 a.m.) Review request for KDE Frameworks. Changes --- add comment, removed useless change. 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 (updated) - cmake/modules/CMakeLists.txt 07f7eac cmake/modules/FindGSSAPI.cmake staging/kio/CMakeLists.txt 6c7297e staging/kio/src/filewidgets/CMakeLists.txt 31fe8c6 staging/kio/src/ioslaves/help/CMakeLists.txt 40637dc 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 tier2/kdoctools/CMakeLists.txt c2256ff tier2/kdoctools/KDocToolsConfig.cmake d501dc8 tier2/kdoctools/KDocToolsConfig.cmake.in PRE-CREATION tier2/kdoctools/src/CMakeLists.txt 3940e98 Diff: http://git.reviewboard.kde.org/r/113723/diff/ Testing --- Builds, Installs, tests still pass; both modularized and monolithic kdelibs. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113795: Cut dependency from KIO to KCoreAddons private headers
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113795/ --- Review request for KDE Frameworks and David Faure. Repository: kdelibs Description --- Make KIO::Job have a separate private class to KJob. It's a bit less optimal, but it will simplify KCoreAddons development. Diffs - staging/kio/src/core/job.cpp 8afc51b staging/kio/src/core/job_p.h 55c00fa staging/kio/src/core/jobclasses.h fb5051c Diff: http://git.reviewboard.kde.org/r/113795/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113723: Fix KIO to build standalone, prepare for moving into its tier
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113723/ --- (Updated Nov. 11, 2013, 2:15 p.m.) Status -- This change has been marked as submitted. 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 - cmake/modules/CMakeLists.txt 07f7eac cmake/modules/FindGSSAPI.cmake staging/kio/CMakeLists.txt 6c7297e staging/kio/src/filewidgets/CMakeLists.txt 31fe8c6 staging/kio/src/ioslaves/help/CMakeLists.txt 40637dc 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 tier2/kdoctools/CMakeLists.txt c2256ff tier2/kdoctools/KDocToolsConfig.cmake d501dc8 tier2/kdoctools/KDocToolsConfig.cmake.in PRE-CREATION tier2/kdoctools/src/CMakeLists.txt 3940e98 Diff: http://git.reviewboard.kde.org/r/113723/diff/ Testing --- Builds, Installs, tests still pass; both modularized and monolithic kdelibs. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113695: Fix KDEWebKit standalone build
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113695/ --- (Updated Nov. 11, 2013, 2:31 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Adds missing dependencies, small other fixes. Diffs - superbuild/CMakeLists.txt 90688f6 tier3/kdewebkit/CMakeLists.txt b56e71d tier3/kdewebkit/src/CMakeLists.txt c48b7ed Diff: http://git.reviewboard.kde.org/r/113695/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113694: Fix KNewStuff standalone build
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113694/ --- (Updated Nov. 11, 2013, 2:31 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Adds dependencies to make sure KNewStuff can be compiled alone Diffs - superbuild/CMakeLists.txt 90688f6 tier3/knewstuff/CMakeLists.txt f7f4dfa Diff: http://git.reviewboard.kde.org/r/113694/diff/ Testing --- Builds and installs. All tests are commented out. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113693: Fix KNotifyConfig standalone build
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113693/ --- (Updated Nov. 11, 2013, 2:31 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Add the dependencies of dependencies. Small fixes. Diffs - superbuild/CMakeLists.txt 90688f6 tier3/knotifyconfig/CMakeLists.txt 8be2ceb tier3/knotifyconfig/tests/CMakeLists.txt 385ff70 Diff: http://git.reviewboard.kde.org/r/113693/diff/ Testing --- Builds, installs, the test seems to work. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113795: Cut dependency from KIO to KCoreAddons private headers
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113795/ --- (Updated Nov. 11, 2013, 6:46 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Repository: kdelibs Description --- Make KIO::Job have a separate private class to KJob. It's a bit less optimal, but it will simplify KCoreAddons development. Diffs - staging/kio/src/core/job.cpp 8afc51b staging/kio/src/core/job_p.h 55c00fa staging/kio/src/core/jobclasses.h fb5051c Diff: http://git.reviewboard.kde.org/r/113795/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113805: Do not change the build types available with cmake
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113805/#review43480 --- Maybe I've missed something, but I would like to have it explained somehow. Why is it bad to define such values? How will g++ calls compare? - Aleix Pol Gonzalez On Nov. 11, 2013, 10:16 p.m., Sune Vuorela wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113805/ --- (Updated Nov. 11, 2013, 10:16 p.m.) Review request for Build System and KDE Frameworks. Repository: extra-cmake-modules Description --- Do not change the build types available with cmake. Diffs - kde-modules/KDECompilerSettings.cmake b034751a5be8073f9628971b552faa079c64e8b6 Diff: http://git.reviewboard.kde.org/r/113805/diff/ Testing --- Built kdelibs on linux with gcc. Thanks, Sune Vuorela ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113805: Do not change the build types available with cmake
On Nov. 12, 2013, 12:02 a.m., Aleix Pol Gonzalez wrote: Maybe I've missed something, but I would like to have it explained somehow. Why is it bad to define such values? How will g++ calls compare? Nicolás Alvarez wrote: In normal CMake, -DCMAKE_BUILD_TYPE=Debug builds without optimization and with debugging symbols. But in KDE projects, Debug uses -O2 -g and there's a custom type called Debugfull which uses -O0 -g. This patch removes all the custom stuff so that Debug means the same (debug, no opts) whether it's a KDE project or not. To get -O2 -g there's RelWithDebInfo. It's also removing the debugfull mode (which is the one that everybody uses, AFAIK), which uses -g3. - Aleix --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113805/#review43480 --- On Nov. 11, 2013, 10:16 p.m., Sune Vuorela wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113805/ --- (Updated Nov. 11, 2013, 10:16 p.m.) Review request for Build System and KDE Frameworks. Repository: extra-cmake-modules Description --- Do not change the build types available with cmake. Diffs - kde-modules/KDECompilerSettings.cmake b034751a5be8073f9628971b552faa079c64e8b6 Diff: http://git.reviewboard.kde.org/r/113805/diff/ Testing --- Built kdelibs on linux with gcc. Thanks, Sune Vuorela ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113810: update kcookiejar manpage
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113810/#review43509 --- Ship it! Looks good to me, probably tasks should be created to review the rest of documentation files. Also some guidelines might be useful as well. - Aleix Pol Gonzalez On Nov. 12, 2013, 10:10 a.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113810/ --- (Updated Nov. 12, 2013, 10:10 a.m.) Review request for KDE Frameworks and David Faure. Repository: kdelibs Description --- update kcookiejar manpage an alternative would be to just remove the kcookiejar binary, I'm not convinced anyone uses it but.. 09:28 Riddell why would you want a command line took to edit your konqueror cookies? 09:42 sandsmark Riddell: why wouldn't you? :D 09:43 sandsmark everything is best from the command line 09:45 agateau :) 09:47 dfaure except food 09:49 agateau well, you get cookies apparently Diffs - staging/kio/docs/CMakeLists.txt d0338c7 staging/kio/docs/kcookiejar5/man-kcookiejar5.8.docbook 0105b12 Diff: http://git.reviewboard.kde.org/r/113810/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113818: Add Remaining Tier 2 Builds to Superbuild
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113818/#review43527 --- Ship it! Ship It! - Aleix Pol Gonzalez On Nov. 12, 2013, 3:03 p.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113818/ --- (Updated Nov. 12, 2013, 3:03 p.m.) Review request for KDE Frameworks and Aurélien Gâteau. Repository: kdelibs Description --- Adding dnssd, kjobwidgets and kwallet. Diffs - superbuild/CMakeLists.txt 53f5952 Diff: http://git.reviewboard.kde.org/r/113818/diff/ Testing --- 1. Configure superbuild for dnssd, kjobwidgets, kwallet and dependencies. 2. Build. Builds successfully. Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113506: Add a sb_all target and sb_$framework targets to make it easier to build frameworks standalone
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113506/#review43533 --- CMakeLists.txt http://git.reviewboard.kde.org/r/113506/#comment31301 I don't think we want that, superbuild shouldn't be used after the splitting but the kde build script. We will need it anyway and it will have all the needed information anyway. I agree that the rest of the changes are good and they should go in, having to enable all modules one by one is annoying. - Aleix Pol Gonzalez On Nov. 7, 2013, 2:42 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113506/ --- (Updated Nov. 7, 2013, 2:42 p.m.) Review request for KDE Frameworks, Alexander Neundorf and Stephen Kelly. Repository: kdelibs Description --- This patch rework superbuild to integrate it more closely with kdelibs build system: one no longer needs to run cmake path/to/kdelibs/superbuild separately. Instead targets are created for all frameworks declared in superbuild/CMakeLists.txt. For example to build and install ki18n standalone, one can run `make sb_ki18n`. It also adds a special sb_all target, which builds and install all standalone frameworks. Diffs - CMakeLists.txt 879fed4 superbuild/CMakeLists.txt cbe9630 superbuild/README 7a4e52e superbuild/SuperBuild.cmake 33ed151 Diff: http://git.reviewboard.kde.org/r/113506/diff/ Testing --- kdelibs still builds standalone, all sb_* targets work as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113821: Don't install kpagedialog_p.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113821/ --- Review request for KDE Frameworks and David Faure. Repository: kdelibs Description --- This makes sure that KWidgetsAddons doesn't expose any private dependencies. The only user of that file was KCMultiDialogPrivate. This patch refactors the code so that it's used separately. Diffs - tier1/kwidgetsaddons/src/CMakeLists.txt 76679a4 tier4/kcmutils/src/kcmultidialog.h 3518736 tier4/kcmutils/src/kcmultidialog.cpp 9d2ccbb tier4/kcmutils/src/kcmultidialog_p.h ad5dd98 Diff: http://git.reviewboard.kde.org/r/113821/diff/ Testing --- Everything builds, couldn't test since I didn't see any test. Tests still pass though. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113683: Fix kdeclarative standalone build
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113683/#review43555 --- Ship it! No, if it works just push it. - Aleix Pol Gonzalez On Nov. 12, 2013, 7:52 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113683/ --- (Updated Nov. 12, 2013, 7:52 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez and Aurélien Gâteau. Repository: kdelibs Description --- Make kdeclarative build standalone. Do I need to add the dependencies of all dependencies as well now? Diffs - superbuild/CMakeLists.txt 56e17dd42a8cc2b2c8a0015c7c5ec74902e0bd3e tier3/kdeclarative/CMakeLists.txt 1f23914a0ba65982c7b597e92fe9c9e920e2abe6 Diff: http://git.reviewboard.kde.org/r/113683/diff/ Testing --- Builds. Thanks, Maarten De Meyer ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113821: Don't install kpagedialog_p.h
On Nov. 13, 2013, 11:53 a.m., David Edmundson wrote: tier4/kcmutils/src/kcmultidialog.cpp, line 262 http://git.reviewboard.kde.org/r/113821/diff/1/?file=213192#file213192line262 I don't think this is right. If we copy a KCMultiDialog instance, we wouldn't copy the KPageDialog d variable of the parent object, but instead create a new one. I think this would be a behavioural change. Well, if I'm not mistaken, it's the same that KPageDialog was doing already: KPageDialog::KPageDialog(KPageDialogPrivate dd, KPageWidget *widget, QWidget *parent, Qt::WindowFlags flags) : QDialog(parent, flags), d_ptr(dd) - Aleix --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113821/#review43573 --- On Nov. 12, 2013, 6:46 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113821/ --- (Updated Nov. 12, 2013, 6:46 p.m.) Review request for KDE Frameworks and David Faure. Repository: kdelibs Description --- This makes sure that KWidgetsAddons doesn't expose any private dependencies. The only user of that file was KCMultiDialogPrivate. This patch refactors the code so that it's used separately. Diffs - tier1/kwidgetsaddons/src/CMakeLists.txt 76679a4 tier4/kcmutils/src/kcmultidialog.h 3518736 tier4/kcmutils/src/kcmultidialog.cpp 9d2ccbb tier4/kcmutils/src/kcmultidialog_p.h ad5dd98 Diff: http://git.reviewboard.kde.org/r/113821/diff/ Testing --- Everything builds, couldn't test since I didn't see any test. Tests still pass though. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113506: Add a sb_all target and sb_$framework targets to make it easier to build frameworks standalone
On Nov. 12, 2013, 5:11 p.m., Aleix Pol Gonzalez wrote: CMakeLists.txt, line 18 http://git.reviewboard.kde.org/r/113506/diff/5/?file=211419#file211419line18 I don't think we want that, superbuild shouldn't be used after the splitting but the kde build script. We will need it anyway and it will have all the needed information anyway. Aurélien Gâteau wrote: I did this to make it as simple as possible to use superbuild: no need to run cmake again, just use the already available targets. When superbuild is removed it can go away with it so I don't think it is a problem. I have 2 kdelibs build directories: one with monolithic and one with superbuild, I don't feel like this is a problem to me. - Aleix --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113506/#review43533 --- On Nov. 13, 2013, 1:07 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113506/ --- (Updated Nov. 13, 2013, 1:07 p.m.) Review request for Build System, KDE Frameworks, Alexander Neundorf, and Stephen Kelly. Repository: kdelibs Description --- This patch rework superbuild to integrate it more closely with kdelibs build system: one no longer needs to run cmake path/to/kdelibs/superbuild separately. Instead targets are created for all frameworks declared in superbuild/CMakeLists.txt. For example to build and install ki18n standalone, one can run `make sb_ki18n`. It also adds a special sb_all target, which builds and install all standalone frameworks. Diffs - CMakeLists.txt 879fed4 superbuild/CMakeLists.txt cbe9630 superbuild/README 7a4e52e superbuild/SuperBuild.cmake 33ed151 Diff: http://git.reviewboard.kde.org/r/113506/diff/ Testing --- kdelibs still builds standalone, all sb_* targets work as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113506: Add a sb_all target and sb_$framework targets to make it easier to build frameworks standalone
On Nov. 12, 2013, 5:11 p.m., Aleix Pol Gonzalez wrote: CMakeLists.txt, line 18 http://git.reviewboard.kde.org/r/113506/diff/5/?file=211419#file211419line18 I don't think we want that, superbuild shouldn't be used after the splitting but the kde build script. We will need it anyway and it will have all the needed information anyway. Aurélien Gâteau wrote: I did this to make it as simple as possible to use superbuild: no need to run cmake again, just use the already available targets. When superbuild is removed it can go away with it so I don't think it is a problem. Aleix Pol Gonzalez wrote: I have 2 kdelibs build directories: one with monolithic and one with superbuild, I don't feel like this is a problem to me. Aurélien Gâteau wrote: Sure it is not, but you made a conscious effort to set it up. Furthermore, not requiring a separate build dir makes like easier for build.kde.org maintainers: they just need to add another target to the make call. Well, I'd say that we probably want to have a separate install for both in bko. I have no idea of how hard this is to set up in jenkins, if the goal is to make it possible for build.kde.org to test it, I won't oppose. - Aleix --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113506/#review43533 --- On Nov. 13, 2013, 1:07 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113506/ --- (Updated Nov. 13, 2013, 1:07 p.m.) Review request for Build System, KDE Frameworks, Alexander Neundorf, and Stephen Kelly. Repository: kdelibs Description --- This patch rework superbuild to integrate it more closely with kdelibs build system: one no longer needs to run cmake path/to/kdelibs/superbuild separately. Instead targets are created for all frameworks declared in superbuild/CMakeLists.txt. For example to build and install ki18n standalone, one can run `make sb_ki18n`. It also adds a special sb_all target, which builds and install all standalone frameworks. Diffs - CMakeLists.txt 879fed4 superbuild/CMakeLists.txt cbe9630 superbuild/README 7a4e52e superbuild/SuperBuild.cmake 33ed151 Diff: http://git.reviewboard.kde.org/r/113506/diff/ Testing --- kdelibs still builds standalone, all sb_* targets work as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113821: Don't install kpagedialog_p.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113821/ --- (Updated Nov. 13, 2013, 3:43 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Repository: kdelibs Description --- This makes sure that KWidgetsAddons doesn't expose any private dependencies. The only user of that file was KCMultiDialogPrivate. This patch refactors the code so that it's used separately. Diffs - tier1/kwidgetsaddons/src/CMakeLists.txt 76679a4 tier4/kcmutils/src/kcmultidialog.h 3518736 tier4/kcmutils/src/kcmultidialog.cpp 9d2ccbb tier4/kcmutils/src/kcmultidialog_p.h ad5dd98 Diff: http://git.reviewboard.kde.org/r/113821/diff/ Testing --- Everything builds, couldn't test since I didn't see any test. Tests still pass though. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113844: Move kspeech interface to tier3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113844/#review43616 --- Why is it tier3? It doesn't seem to depend on anything, no? - Aleix Pol Gonzalez On Nov. 13, 2013, 5:16 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113844/ --- (Updated Nov. 13, 2013, 5:16 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- It builds and installs. Will make it conform to framework structure with src/ and whatnot after this is shipped. Diffs - interfaces/CMakeLists.txt 9fa4e1b23e4671dae25518c9d8d3fe1369f9b1a6 interfaces/kspeech/CMakeLists.txt interfaces/kspeech/Mainpage.dox interfaces/kspeech/dbustexttospeech.desktop interfaces/kspeech/kspeech.h interfaces/kspeech/org.kde.KSpeech.xml tier3/CMakeLists.txt c801fd299d95f64d28676f80e86e2c86f9b39024 Diff: http://git.reviewboard.kde.org/r/113844/diff/ Testing --- Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113844: Move kspeech interface to tier3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113844/#review43621 --- Ship it! Ship It! - Aleix Pol Gonzalez On Nov. 13, 2013, 6:01 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113844/ --- (Updated Nov. 13, 2013, 6:01 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- It builds and installs. Will make it conform to framework structure with src/ and whatnot after this is shipped. Diffs - interfaces/CMakeLists.txt 9fa4e1b23e4671dae25518c9d8d3fe1369f9b1a6 interfaces/kspeech/CMakeLists.txt interfaces/kspeech/Mainpage.dox interfaces/kspeech/dbustexttospeech.desktop interfaces/kspeech/kspeech.h interfaces/kspeech/org.kde.KSpeech.xml staging/CMakeLists.txt c77fa19ad700c6e9af68434d7c6fdc304fceeba6 Diff: http://git.reviewboard.kde.org/r/113844/diff/ Testing --- Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113867: Add knewstuff public dependencies to it's config.cmake file.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113867/#review43690 --- Ship it! Ship It! - Aleix Pol Gonzalez On Nov. 14, 2013, 6:03 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113867/ --- (Updated Nov. 14, 2013, 6:03 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- KNewStuff's Config.cmake is missing it's public dependencies. This adds them. Diffs - tier3/knewstuff/KNewStuffConfig.cmake.in 54bee483444919ee0a337d117ff5f48139d04359 Diff: http://git.reviewboard.kde.org/r/113867/diff/ Testing --- Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113881: Standardise d-pointer classes of kmediaplayer
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113881/#review43726 --- Looks good to me. Actually I don't really understand what the Data pointer was before. It's only defined for one class but both have it as a member? It was used as a d-ptr only for some members? - Aleix Pol Gonzalez On Nov. 15, 2013, 12:20 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113881/ --- (Updated Nov. 15, 2013, 12:20 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- Standardise d-pointer classes of kmediaplayer Use class Private instead of struct Data, for consistency with other KDE software, and move all private members to those classes. Diffs - staging/kmediaplayer/src/kmediaplayer/view.h 3bc849f5d93b5bb04c811c0c598d1f27fb25bfc2 staging/kmediaplayer/src/kmediaplayer/player.cpp a4618c4b1d93e2203cea496c100a0ed4ebe4d8dd staging/kmediaplayer/src/kmediaplayer/player.h c59c06fca0ea22994a7f00c6c509777be09370f1 staging/kmediaplayer/src/kmediaplayer/view.cpp eb2b779e8a4b044c1aa4f57b15567688da51a626 Diff: http://git.reviewboard.kde.org/r/113881/diff/ Testing --- Still builds. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113882: Make constructors in KMediaPlayer explicit
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113882/#review43728 --- Ship it! Ship It! - Aleix Pol Gonzalez On Nov. 15, 2013, 12:21 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113882/ --- (Updated Nov. 15, 2013, 12:21 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- Make constructors in KMediaPlayer explicit Diffs - staging/kmediaplayer/src/kfileaudiopreview/kfileaudiopreview.h 9f76454e013f95d8905b9c40679f7bcd755f949b staging/kmediaplayer/src/kfileaudiopreview/mediacontrols.h 28151efd06ac460446cbf5764239fecfbeff5356 staging/kmediaplayer/src/kmediaplayer/player.h c59c06fca0ea22994a7f00c6c509777be09370f1 staging/kmediaplayer/src/kmediaplayer/view.h 3bc849f5d93b5bb04c811c0c598d1f27fb25bfc2 Diff: http://git.reviewboard.kde.org/r/113882/diff/ Testing --- Builds. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113883: Rename translation catalogue kfileaudiopreview4 to kfileaudiopreview
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113883/#review43729 --- Why do you need to drop the version? In similar cases we've been bumping to 5... - Aleix Pol Gonzalez On Nov. 15, 2013, 12:22 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113883/ --- (Updated Nov. 15, 2013, 12:22 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- Rename translation catalogue kfileaudiopreview4 to kfileaudiopreview Diffs - staging/kmediaplayer/src/kfileaudiopreview/Messages.sh 71c7067f3431c1d6d7e920285f1e7382d4218d2d staging/kmediaplayer/src/kfileaudiopreview/kfileaudiopreview.cpp 04efb249caca5639e3531b092681ce7fba0e8b05 staging/kmediaplayer/src/kfileaudiopreview/mediacontrols_p.h bb36ab1bada192e8f80b217fe782b1a96016ada3 Diff: http://git.reviewboard.kde.org/r/113883/diff/ Testing --- Builds. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113887: Deprecate KDE4_* KAuth macros
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113887/ --- Review request for Build System and KDE Frameworks. Repository: kdelibs Description --- Prefix them with kauth_* instead, also remove assumptions from kde4_*. Diffs - tier4/kde4support/CreateKDELibsDependenciesFile.cmake 55fb337 tier2/kauth/src/kauthhelpersupport.h a9d7a53 tier2/kauth/src/kauthactionreply.h a91e667 tier2/kauth/src/kauthaction.h 07a5506 tier2/kauth/src/ConfigureChecks.cmake f00f2fe tier2/kauth/src/CMakeLists.txt f61eee5 tier2/kauth/KAuthConfig.cmake.in dee448b tier2/kauth/cmake/KAuthMacros.cmake d942d69 tier4/kde4support/cmake/modules/KDE4Macros.cmake 6a63e5e Diff: http://git.reviewboard.kde.org/r/113887/diff/ Testing --- Nothing stopped building. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113863: Remove WINCE specific cmake code paths
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113863/ --- (Updated Nov. 15, 2013, 5:35 p.m.) Review request for Build System and KDE Frameworks. Changes --- added buildsystem group, hoping somebody will see it. Repository: kdelibs Description --- Removes WINCE specific code. It's not a supported platform anymore. Diffs - tier1/kcoreaddons/src/lib/CMakeLists.txt 53e85e8 tier1/kcoreaddons/src/lib/util/kuser_wince.cpp b96e4dc tier3/kinit/src/kdeinit/CMakeLists.txt bfa8659 tier3/kio/src/CMakeLists.txt 0b12bb8 tier3/kprintutils/src/CMakeLists.txt 7436f91 tier4/kde4support/src/CMakeLists.txt 7aec195 Diff: http://git.reviewboard.kde.org/r/113863/diff/ Testing --- still builds. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113887: Deprecate KDE4_* KAuth macros
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113887/ --- (Updated Nov. 18, 2013, 2:22 p.m.) Status -- This change has been marked as submitted. Review request for Build System and KDE Frameworks. Repository: kdelibs Description --- Prefix them with kauth_* instead, also remove assumptions from kde4_*. Diffs - tier4/kde4support/CreateKDELibsDependenciesFile.cmake 55fb337 tier2/kauth/src/kauthhelpersupport.h a9d7a53 tier2/kauth/src/kauthactionreply.h a91e667 tier2/kauth/src/kauthaction.h 07a5506 tier2/kauth/src/ConfigureChecks.cmake f00f2fe tier2/kauth/src/CMakeLists.txt f61eee5 tier2/kauth/KAuthConfig.cmake.in dee448b tier2/kauth/cmake/KAuthMacros.cmake d942d69 tier4/kde4support/cmake/modules/KDE4Macros.cmake 6a63e5e Diff: http://git.reviewboard.kde.org/r/113887/diff/ Testing --- Nothing stopped building. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113863: Remove WINCE specific cmake code paths
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113863/ --- (Updated Nov. 18, 2013, 2:34 p.m.) Review request for Build System and KDE Frameworks. Repository: kdelibs Description --- Removes WINCE specific code. It's not a supported platform anymore. Diffs (updated) - tier1/kcoreaddons/src/lib/CMakeLists.txt a1f81f7 tier1/kcoreaddons/src/lib/util/kuser_wince.cpp b96e4dc tier3/kinit/src/kdeinit/CMakeLists.txt 4ef0a9b tier3/kio/src/CMakeLists.txt 3800e85 tier3/kprintutils/src/CMakeLists.txt da0b7d5 tier4/kde4support/src/CMakeLists.txt 9046c96 Diff: http://git.reviewboard.kde.org/r/113863/diff/ Testing --- still builds. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113972: Add some autotests for KMediaPlayer
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113972/#review44084 --- Ship it! Ship It! - Aleix Pol Gonzalez On Nov. 21, 2013, midnight, Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113972/ --- (Updated Nov. 21, 2013, midnight) Review request for KDE Frameworks. Repository: kdelibs Description --- Add some autotests for KMediaPlayer Diffs - staging/kmediaplayer/CMakeLists.txt c9f301ae961a0fd7f4ad71f1a7cec766493f3d14 staging/kmediaplayer/autotests/CMakeLists.txt PRE-CREATION staging/kmediaplayer/autotests/playertest.cpp PRE-CREATION staging/kmediaplayer/autotests/testplayer.h PRE-CREATION staging/kmediaplayer/autotests/testview.h PRE-CREATION staging/kmediaplayer/autotests/viewtest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/113972/diff/ Testing --- Builds, new tests run and pass. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113996: Install needed private headers for FrameworksIntegration
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113996/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- There are 2 headers to install: - kmessageboxnotifyinterface.h: It's an interface, it's obvious to me that it should be installed - kiconengine_p.h: I have no idea, but it's still needed. (note it's also needed by kicon.cpp in kde4support). Diffs - tier1/kwidgetsaddons/src/CMakeLists.txt 9223ccf tier3/kiconthemes/src/CMakeLists.txt 4b3c978 Diff: http://git.reviewboard.kde.org/r/113996/diff/ Testing --- FrameworksIntegration builds modularized, monolithic kdelibs still build as well. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 114003: Install private headers needed by kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114003/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- defaultviewadapter_p.h: Needed by tier4/kde4support/src/kio/kmimetyperesolver.cpp kfiletreeview_p.h: Needed by tier4/kde4support/src/kio/kdirselectdialog.cpp Diffs - tier3/kio/src/filewidgets/CMakeLists.txt 13a3918 Diff: http://git.reviewboard.kde.org/r/114003/diff/ Testing --- Builds, tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 114038: Integrate KAboutData and QCommandLineParser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114038/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- There was a functionality regression from KDE4 where every application would get an --authors and a --license argument automatically. This patch makes it possible to let KAboutData configure QCommandLineParser to add the missing arguments and react if those are required. Diffs - tier1/kcoreaddons/src/lib/kaboutdata.h f9dee8e tier1/kcoreaddons/src/lib/kaboutdata.cpp 5ea8862 tier3/kservice/src/kbuildsycoca/CMakeLists.txt 52b46c4 tier3/kservice/src/kbuildsycoca/kbuildsycoca.cpp 05019ad Diff: http://git.reviewboard.kde.org/r/114038/diff/ Testing --- Made kbuildsycoca5 use it and made sure it works. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114003: Install private headers needed by kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114003/ --- (Updated Nov. 22, 2013, 6:23 p.m.) Review request for KDE Frameworks. Changes --- Remove kmimetyperesolver.cpp so that we don't need to install defaultviewadapter_p.h. Now we're not installing any new header files. \o/ Repository: kdelibs Description --- defaultviewadapter_p.h: Needed by tier4/kde4support/src/kio/kmimetyperesolver.cpp kfiletreeview_p.h: Needed by tier4/kde4support/src/kio/kdirselectdialog.cpp Diffs (updated) - tier3/kio/src/filewidgets/CMakeLists.txt 13a3918 tier3/kio/src/filewidgets/kfiletreeview.cpp tier3/kio/src/filewidgets/kfiletreeview_p.h d8a8aa6 tier3/kio/tests/CMakeLists.txt 7086285 tier3/kio/tests/kfiletreeviewtest.h tier3/kio/tests/kfiletreeviewtest.cpp tier4/kde4support/src/CMakeLists.txt 9a731fd tier4/kde4support/src/kio/kmimetyperesolver.h 515dcfb tier4/kde4support/src/kio/kmimetyperesolver.cpp 69e64fd tier4/kde4support/tests/CMakeLists.txt 7705381 Diff: http://git.reviewboard.kde.org/r/114003/diff/ Testing --- Builds, tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113975: Make some KSSL files public API
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113975/ --- (Updated Nov. 25, 2013, 11:39 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kdelibs Description --- I was trying to get KHTML to build standalone, the only missing part I have is these files need to be public API and in the correct namespace. Diffs - tier3/kio/src/core/CMakeLists.txt 1944435 Diff: http://git.reviewboard.kde.org/r/113975/diff/ Testing --- Everything still builds. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114003: Install private headers needed by kde4support
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114003/ --- (Updated Nov. 25, 2013, 12:26 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- defaultviewadapter_p.h: Needed by tier4/kde4support/src/kio/kmimetyperesolver.cpp kfiletreeview_p.h: Needed by tier4/kde4support/src/kio/kdirselectdialog.cpp Diffs - tier3/kio/src/filewidgets/CMakeLists.txt 13a3918 tier3/kio/src/filewidgets/kfiletreeview.cpp tier3/kio/src/filewidgets/kfiletreeview_p.h d8a8aa6 tier3/kio/tests/CMakeLists.txt 7086285 tier3/kio/tests/kfiletreeviewtest.h tier3/kio/tests/kfiletreeviewtest.cpp tier4/kde4support/src/CMakeLists.txt 9a731fd tier4/kde4support/src/kio/kmimetyperesolver.h 515dcfb tier4/kde4support/src/kio/kmimetyperesolver.cpp 69e64fd tier4/kde4support/tests/CMakeLists.txt 7705381 Diff: http://git.reviewboard.kde.org/r/114003/diff/ Testing --- Builds, tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113996: Install needed private headers for FrameworksIntegration
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113996/ --- (Updated Nov. 25, 2013, 12:26 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- There are 2 headers to install: - kmessageboxnotifyinterface.h: It's an interface, it's obvious to me that it should be installed - kiconengine_p.h: I have no idea, but it's still needed. (note it's also needed by kicon.cpp in kde4support). Diffs - tier1/kwidgetsaddons/src/CMakeLists.txt 9223ccf tier3/kiconthemes/src/CMakeLists.txt 4b3c978 tier3/kiconthemes/src/kiconengine_p.h Diff: http://git.reviewboard.kde.org/r/113996/diff/ Testing --- FrameworksIntegration builds modularized, monolithic kdelibs still build as well. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114038: Integrate KAboutData and QCommandLineParser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114038/ --- (Updated Nov. 25, 2013, 4:26 p.m.) Review request for KDE Frameworks. Changes --- adopted david's suggestions Repository: kdelibs Description --- There was a functionality regression from KDE4 where every application would get an --authors and a --license argument automatically. This patch makes it possible to let KAboutData configure QCommandLineParser to add the missing arguments and react if those are required. Diffs (updated) - tier1/kcoreaddons/src/lib/kaboutdata.h b56832c tier1/kcoreaddons/src/lib/kaboutdata.cpp 5ea8862 tier3/kservice/src/kbuildsycoca/CMakeLists.txt 52b46c4 tier3/kservice/src/kbuildsycoca/kbuildsycoca.cpp 05019ad Diff: http://git.reviewboard.kde.org/r/114038/diff/ Testing --- Made kbuildsycoca5 use it and made sure it works. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114038: Integrate KAboutData and QCommandLineParser
On Nov. 24, 2013, 10:58 a.m., David Faure wrote: tier1/kcoreaddons/src/lib/kaboutdata.cpp, line 957 http://git.reviewboard.kde.org/r/114038/diff/1/?file=219248#file219248line957 (LOL, most likely someone forgot to call addAuthor, but OK :) Wrong usage of tr() (this will use QCoreApplication as context). Use tr() (might need Q_DECLARE_TR_FUNCTIONS in the header, since this isn't a QObject), or use translate() with a context, like in the example program in this patch. I used the same we used to have in KCmdLineArgs. I agree it looks a bit aggressive though :). As if there was any kind of anonymity going on... I'll fix the ::tr usage. - Aleix --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114038/#review44296 --- On Nov. 22, 2013, 6:07 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114038/ --- (Updated Nov. 22, 2013, 6:07 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- There was a functionality regression from KDE4 where every application would get an --authors and a --license argument automatically. This patch makes it possible to let KAboutData configure QCommandLineParser to add the missing arguments and react if those are required. Diffs - tier1/kcoreaddons/src/lib/kaboutdata.h f9dee8e tier1/kcoreaddons/src/lib/kaboutdata.cpp 5ea8862 tier3/kservice/src/kbuildsycoca/CMakeLists.txt 52b46c4 tier3/kservice/src/kbuildsycoca/kbuildsycoca.cpp 05019ad Diff: http://git.reviewboard.kde.org/r/114038/diff/ Testing --- Made kbuildsycoca5 use it and made sure it works. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114272: Fix broken connect for signal finished in kcmultidialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114272/#review45042 --- Ship it! Ship It! - Aleix Pol Gonzalez On Dec. 3, 2013, 2:34 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114272/ --- (Updated Dec. 3, 2013, 2:34 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- see title Diffs - tier3/kcmutils/src/kcmultidialog.cpp 5326b69 Diff: http://git.reviewboard.kde.org/r/114272/diff/ Testing --- Without change: warning when starting kcmshell5 With change: no warning when starting kcmshell5 Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114256: Make QFileDialog use KFileWidget from the KDE PlatformTheme
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114256/ --- (Updated Dec. 4, 2013, 6:09 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Since I'd say it's the most important dialog, I decided to adopt it right away. Instead of moving KFileDialog or anything, I chose to create a new dialog that just contains a KFileWidget, which is what I understand what we want in a last instance. Everything is deeply undocumented, so if anybody has a clue about QPlatformTheme, it would be interesting if s/he could take a look. Diffs - tier4/frameworkintegration/tests/CMakeLists.txt 4ed65f8 tier4/frameworkintegration/src/platformtheme/kdeplatformtheme.cpp 010eed4 tier4/frameworkintegration/src/platformtheme/kdeplatformtheme.h 69f1f44 tier4/frameworkintegration/src/platformtheme/kdeplatformfiledialoghelper.cpp PRE-CREATION tier4/frameworkintegration/src/platformtheme/kdeplatformfiledialoghelper.h PRE-CREATION tier4/frameworkintegration/src/platformtheme/CMakeLists.txt 5d95c8a tier4/frameworkintegration/autotests/CMakeLists.txt f126859 tier4/frameworkintegration/CMakeLists.txt 02b3563 tier4/frameworkintegration/tests/qfiledialogtest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/114256/diff/ Testing --- Created a test app that instantiates a QFileDialog, used a QFileDialog from systemsettings. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114185: Provide our own KFontDialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114185/ --- (Updated Dec. 8, 2013, 5:50 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kdelibs Description --- To brighten a bit further the e-mail I just sent to kde-framewo...@kde.org, here I'm making the PlatformTheme to use our KFontDialog, that now it's residing in KDE4Support. I'd say this should result in a new module created, since we don't want to depend on KDE4Support, although I can see how we don't want to maintain an ABI on those classes, so forking is an option too. Diffs - tier4/frameworkintegration/CMakeLists.txt 02b3563 tier4/frameworkintegration/autotests/CMakeLists.txt f126859 tier4/frameworkintegration/src/platformtheme/CMakeLists.txt 5d95c8a tier4/frameworkintegration/src/platformtheme/kdeplatformfontdialoghelper.h PRE-CREATION tier4/frameworkintegration/src/platformtheme/kdeplatformfontdialoghelper.cpp PRE-CREATION tier4/frameworkintegration/src/platformtheme/kdeplatformtheme.h 69f1f44 tier4/frameworkintegration/src/platformtheme/kdeplatformtheme.cpp 010eed4 Diff: http://git.reviewboard.kde.org/r/114185/diff/ Testing --- Works with the kfontrequestertest. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114356: Move KGlobalAccel from XmlGui into an own tier1 framework
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114356/#review45465 --- tier1/kglobalaccel/KF5GlobalAccelConfig.cmake.in http://git.reviewboard.kde.org/r/114356/#comment32420 Shouldn't that file be called KGlobalAccelConfig.cmake.in? We're not prefixing the rest of modules with KF5, right? - Aleix Pol Gonzalez On Dec. 10, 2013, 6:41 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114356/ --- (Updated Dec. 10, 2013, 6:41 a.m.) Review request for KDE Frameworks and Martin Klapetek. Repository: kdelibs Description --- New framework needs DBus, Widgets and (optionally) X11Extras as dependencies. @MartinK: how should we organize the renaming? I'm still using the old naming scheme. So should I adjust or should we use your scripts after pulling this one in? Diffs - tier1/CMakeLists.txt 450ca57 tier1/kglobalaccel/CMakeLists.txt PRE-CREATION tier1/kglobalaccel/KF5GlobalAccelConfig.cmake.in PRE-CREATION tier1/kglobalaccel/autotests/CMakeLists.txt PRE-CREATION tier1/kglobalaccel/autotests/kglobalshortcuttest.h PRE-CREATION tier1/kglobalaccel/autotests/kglobalshortcuttest.cpp PRE-CREATION tier1/kglobalaccel/src/CMakeLists.txt PRE-CREATION tier1/kglobalaccel/src/config-kglobalaccel.h.cmake PRE-CREATION tier1/kglobalaccel/src/kglobalaccel.h PRE-CREATION tier1/kglobalaccel/src/kglobalaccel.cpp PRE-CREATION tier1/kglobalaccel/src/kglobalaccel_p.h PRE-CREATION tier1/kglobalaccel/src/kglobalshortcutinfo.h PRE-CREATION tier1/kglobalaccel/src/kglobalshortcutinfo.cpp PRE-CREATION tier1/kglobalaccel/src/kglobalshortcutinfo_dbus.cpp PRE-CREATION tier1/kglobalaccel/src/kglobalshortcutinfo_p.h PRE-CREATION tier1/kglobalaccel/src/org.kde.KGlobalAccel.xml PRE-CREATION tier1/kglobalaccel/src/org.kde.kglobalaccel.Component.xml PRE-CREATION tier3/xmlgui/CMakeLists.txt 75593fb tier3/xmlgui/autotests/CMakeLists.txt 6546ac7 tier3/xmlgui/autotests/kglobalshortcuttest.h d8e0024 tier3/xmlgui/autotests/kglobalshortcuttest.cpp 453920b tier3/xmlgui/src/CMakeLists.txt 31621e2 tier3/xmlgui/src/kactioncollection.cpp 0266069 tier3/xmlgui/src/kglobalaccel.h 57482ad tier3/xmlgui/src/kglobalaccel.cpp 77ce993 tier3/xmlgui/src/kglobalaccel_p.h 50f271f tier3/xmlgui/src/kglobalshortcutinfo.h 0920f01 tier3/xmlgui/src/kglobalshortcutinfo.cpp d2ca948 tier3/xmlgui/src/kglobalshortcutinfo_dbus.cpp f49bd9d tier3/xmlgui/src/kglobalshortcutinfo_p.h dd2dd2d tier3/xmlgui/src/kkeysequencewidget.cpp 2148ce0 tier3/xmlgui/src/kshortcutseditor.cpp 3e917d2 tier3/xmlgui/src/kshortcutseditoritem.cpp e7257a6 tier3/xmlgui/src/kxmlguifactory.cpp 510d2ea tier3/xmlgui/src/org.kde.KGlobalAccel.xml 8746551 tier3/xmlgui/src/org.kde.kglobalaccel.Component.xml ec21201 tier4/kde4support/CMakeLists.txt cb73b46 tier4/kde4support/src/CMakeLists.txt 39298b9 tier4/khtml/CMakeLists.txt c937f38 tier4/khtml/src/CMakeLists.txt 00243fb Diff: http://git.reviewboard.kde.org/r/114356/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112463: Port SMB kioslave to KF5/Qt5
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112463/#review45466
---
kioslave/smb/CMakeLists.txt
http://git.reviewboard.kde.org/r/112463/#comment32421
this should be:
add_library(kio_smb MODULE {kio_smb_PART_SRCS})
kioslave/smb/CMakeLists.txt
http://git.reviewboard.kde.org/r/112463/#comment32422
also add_library.
kioslave/smb/kio_smb.h
http://git.reviewboard.kde.org/r/112463/#comment32423
do you need klocale? it's deprecated.
kioslave/smb/kio_smb_auth.cpp
http://git.reviewboard.kde.org/r/112463/#comment32424
A
xD
Maybe we can just remove this one?
kioslave/smb/kio_smb_internal.h
http://git.reviewboard.kde.org/r/112463/#comment32426
it might be a good moment to remove this comment-
kioslave/smb/kio_smb_internal.cpp
http://git.reviewboard.kde.org/r/112463/#comment32427
Does it really need QDir::separator()? It seems to me like everything
should be '/'.
- Aleix Pol Gonzalez
On Dec. 5, 2013, 11:44 p.m., Mark Gaiser wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112463/
---
(Updated Dec. 5, 2013, 11:44 p.m.)
Review request for KDE Runtime and KDE Frameworks.
Repository: kde-runtime
Description
---
This is the initial port! I added two TODO lines in the diff for parts where
i'm not sure if I've ported them correctly.
Also, i needed a change in FindSamba.cmake to even get the samba detection
working. That reviewrequest is waiting here:
https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use
samba 3.x
Once i know that this is actually working then i will comment some qDebug
lines.
Diffs
-
kioslave/CMakeLists.txt fc594e4
kioslave/smb/CMakeLists.txt a3a2265
kioslave/smb/kio_smb.h c2229ab
kioslave/smb/kio_smb.cpp 2c2523a
kioslave/smb/kio_smb_auth.cpp 4d236b4
kioslave/smb/kio_smb_browse.cpp 5253be9
kioslave/smb/kio_smb_dir.cpp ba80c86
kioslave/smb/kio_smb_file.cpp 206526a
kioslave/smb/kio_smb_internal.h 4b946c1
kioslave/smb/kio_smb_internal.cpp e943844
kioslave/smb/kio_smb_mount.cpp a5a7e8e
kioslave/smb/kio_smb_win.h f1dcb6f
kioslave/smb/kio_smb_win.cpp 14dd797
Diff: http://git.reviewboard.kde.org/r/112463/diff/
Testing
---
It compiles and gets loaded just fine. I tried testing this on an actual
samba share, but i kept getting a 111 error (connection refused) from kio_smb
so i'm hoping that is a local issue here. If someone else could try this out
and verify that it's either working or broken.
Thanks,
Mark Gaiser
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112463: Port SMB kioslave to KF5/Qt5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112463/#review45520 --- Ship it! - Aleix Pol Gonzalez On Dec. 10, 2013, 6:51 p.m., Mark Gaiser wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112463/ --- (Updated Dec. 10, 2013, 6:51 p.m.) Review request for KDE Runtime and KDE Frameworks. Repository: kde-runtime Description --- This is the initial port! I added two TODO lines in the diff for parts where i'm not sure if I've ported them correctly. Also, i needed a change in FindSamba.cmake to even get the samba detection working. That reviewrequest is waiting here: https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use samba 3.x Once i know that this is actually working then i will comment some qDebug lines. Diffs - kioslave/CMakeLists.txt fc594e4 kioslave/smb/CMakeLists.txt a3a2265 kioslave/smb/kio_smb.h c2229ab kioslave/smb/kio_smb.cpp 2c2523a kioslave/smb/kio_smb_auth.cpp 4d236b4 kioslave/smb/kio_smb_browse.cpp 5253be9 kioslave/smb/kio_smb_dir.cpp ba80c86 kioslave/smb/kio_smb_file.cpp 206526a kioslave/smb/kio_smb_internal.h 4b946c1 kioslave/smb/kio_smb_internal.cpp e943844 kioslave/smb/kio_smb_mount.cpp a5a7e8e kioslave/smb/kio_smb_win.h f1dcb6f kioslave/smb/kio_smb_win.cpp 14dd797 Diff: http://git.reviewboard.kde.org/r/112463/diff/ Testing --- It compiles and gets loaded just fine. I tried testing this on an actual samba share, but i kept getting a 111 error (connection refused) from kio_smb so i'm hoping that is a local issue here. If someone else could try this out and verify that it's either working or broken. Thanks, Mark Gaiser ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114447: Split Frameworks only definitions to a new file
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114447/#review45683 --- Ship it! Ship It! - Aleix Pol Gonzalez On Dec. 14, 2013, 3:29 p.m., Albert Astals Cid wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114447/ --- (Updated Dec. 14, 2013, 3:29 p.m.) Review request for KDE Frameworks and Aleix Pol Gonzalez. Repository: extra-cmake-modules Description --- Don't force this extra correctness Qt flags on the rest of the people, let them decide by themselves. Diffs - kde-modules/KDECompilerSettings.cmake 5f2da5e kde-modules/KDEFrameworkCompilerSettings.cmake PRE-CREATION Diff: http://git.reviewboard.kde.org/r/114447/diff/ Testing --- Installed, change kdelibs to use the new file instead of the old, all was the same. Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114501: Rename KDE4_ENABLE_EXCEPTIONS to KDE_ENABLE_EXCEPTIONS
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114501/#review45802 --- Ship it! Ship It! - Aleix Pol Gonzalez On Dec. 16, 2013, 5:10 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114501/ --- (Updated Dec. 16, 2013, 5:10 p.m.) Review request for Build System and KDE Frameworks. Repository: extra-cmake-modules Description --- Rename KDE4_ENABLE_EXCEPTIONS to KDE_ENABLE_EXCEPTIONS. One less KDE4 reference. I have a matching patch for kdelibs once this goes in. When the kdelibs patch is in, the temporary definition of KDE4_ENABLE_EXCEPTIONS can be removed. Would be good to also factorize the definition and decide once and for all if it should include -UQT_NO_EXCEPTIONS, but that's for another review. kdelibs patch: https://git.reviewboard.kde.org/r/114502/ Diffs - kde-modules/KDECompilerSettings.cmake 6dafc5f Diff: http://git.reviewboard.kde.org/r/114501/diff/ Testing --- kdelibs build. Checked with `make VERBOSE=1` the exception flags are still correctly passed. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114502: KDE4_ENABLE_EXCEPTIONS = KDE_ENABLE_EXCEPTIONS
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114502/#review45801 --- Ship it! Looks good to me. - Aleix Pol Gonzalez On Dec. 16, 2013, 5:10 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114502/ --- (Updated Dec. 16, 2013, 5:10 p.m.) Review request for Build System and KDE Frameworks. Repository: kdelibs Description --- Use the new KDE_ENABLE_EXCEPTIONS flag, define KDE4_ENABLE_EXCEPTIONS in KDE4Support. Needs https://git.reviewboard.kde.org/r/114501/ Diffs - tier4/khtml/src/CMakeLists.txt a8eb322 tier4/kde4support/cmake/modules/FindKDE4Internal.cmake e3f8c63 tier1/threadweaver/src/Weaver/CMakeLists.txt 8e18b46 tier3/kio/src/kpac/CMakeLists.txt 57d5694 tier1/kimageformats/src/imageformats/CMakeLists.txt 614bba0 Diff: http://git.reviewboard.kde.org/r/114502/diff/ Testing --- kdelibs builds, even after removing the KDE4_ENABLE_EXCEPTIONS definition from ECM (see ECM review). Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114683: Clean up target_link_libraries for KF5Plasma
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114683/#review46193 --- Ship it! Ship It! - Aleix Pol Gonzalez On Dec. 27, 2013, 2:43 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114683/ --- (Updated Dec. 27, 2013, 2:43 p.m.) Review request for Build System, KDE Frameworks and Plasma. Repository: plasma-framework Description --- Clean up target_link_libraries for KF5Plasma It is now a single call using PUBLIC and PRIVATE keywords. This removes a CMake warning about using LINK_INTERFACE_LIBRARIES. Diffs - src/plasma/CMakeLists.txt 5a6ceabb3d6b95fb792e19aab034b87d0dd688a9 Diff: https://git.reviewboard.kde.org/r/114683/diff/ Testing --- plasma-framework and kde-workspace both still compile. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114716/#review46422 --- Ship it! Ship It! - Aleix Pol Gonzalez On Dec. 29, 2013, 11:50 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114716/ --- (Updated Dec. 29, 2013, 11:50 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Without this patch, the parsing of kgeography --author gives: kgeography was written by: Albert Astals cidaa...@kde.org With this patch: kgeography was written by: Albert Astals Cidaa...@kde.org Diffs - src/lib/kaboutdata.cpp 3f08f25 Diff: https://git.reviewboard.kde.org/r/114716/diff/ Testing --- It builds. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114937: port kconfig_compiler_kf5 to QCommandLineParser
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114937/#review47143 --- src/kconfig_compiler/kconfig_compiler.cpp https://git.reviewboard.kde.org/r/114937/#comment33590 directoryName is not used now? - Aleix Pol Gonzalez On Jan. 10, 2014, 4:03 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114937/ --- (Updated Jan. 10, 2014, 4:03 p.m.) Review request for KDE Frameworks and David Faure. Repository: kconfig Description --- $summary Diffs - src/kconfig_compiler/kconfig_compiler.cpp df17d4c Diff: https://git.reviewboard.kde.org/r/114937/diff/ Testing --- Thanks, Bhushan Shah ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115016: Make KJob usable from QML
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115016/ --- Review request for KDE Frameworks. Repository: kcoreaddons Description --- Add a couple of Q_PROPERTY and Q_SCRIPTABLE to KJob, so that it can be consumed elsewhere. The idea isn't to be able to implement KJobs, but to be able to keep track of them from QML and check if it succeeded. Diffs - src/lib/jobs/kjob.h 24dbaec Diff: https://git.reviewboard.kde.org/r/115016/diff/ Testing --- Everything still builds, shouldn't be a big deal. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115016: Make KJob usable from QML
On Jan. 14, 2014, 8:20 p.m., Alex Merry wrote: src/lib/jobs/kjob.h, line 92 https://git.reviewboard.kde.org/r/115016/diff/1/?file=233991#file233991line92 I don't think this will work; I'm fairly sure that notify signals must have zero or one arguments, and the one argument must be the same type as the property. The notify signal in this class has a preceding KJob* argument. Quoting the documentation: A NOTIFY signal is optional. If defined, it should specify one existing signal in that class that is emitted whenever the value of the property changes. Also I've been using this in a plasmoid I'm porting and it doesn't seem to cause problems. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115016/#review47395 --- On Jan. 14, 2014, 8:16 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115016/ --- (Updated Jan. 14, 2014, 8:16 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Add a couple of Q_PROPERTY and Q_SCRIPTABLE to KJob, so that it can be consumed elsewhere. The idea isn't to be able to implement KJobs, but to be able to keep track of them from QML and check if it succeeded. Diffs - src/lib/jobs/kjob.h 24dbaec Diff: https://git.reviewboard.kde.org/r/115016/diff/ Testing --- Everything still builds, shouldn't be a big deal. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115030: Install public headers for KJsEmbed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115030/ --- Review request for KDE Frameworks. Repository: kjsembed Description --- KJsEmbed library is useless without headers installed. This patch installs them. Diffs - CMakeLists.txt fc1731a src/kjsembed/CMakeLists.txt b4d3f5f Diff: https://git.reviewboard.kde.org/r/115030/diff/ Testing --- I ported the share dataengine to use it (it was using the now defunct Kross KJS support). https://git.reviewboard.kde.org/r/115027/ Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115016: Make KJob usable from QML
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115016/ --- (Updated Jan. 16, 2014, 11:06 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcoreaddons Description --- Add a couple of Q_PROPERTY and Q_SCRIPTABLE to KJob, so that it can be consumed elsewhere. The idea isn't to be able to implement KJobs, but to be able to keep track of them from QML and check if it succeeded. Diffs - src/lib/jobs/kjob.h 24dbaec Diff: https://git.reviewboard.kde.org/r/115016/diff/ Testing --- Everything still builds, shouldn't be a big deal. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115049: Do not require ktranscript to be installed for the test to run
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115049/#review47490
---
autotests/ktranscripttest.cpp
https://git.reviewboard.kde.org/r/115049/#comment33759
This limits the cwd. Maybe you can pass it as a preprocess definition?
aka with add_definitions(-DKTRANSCRIPT_PATH=${blah}) ).
- Aleix Pol Gonzalez
On Jan. 16, 2014, 11:07 a.m., Aurélien Gâteau wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115049/
---
(Updated Jan. 16, 2014, 11:07 a.m.)
Review request for KDE Frameworks and Chusslove Illich.
Bugs: 329994
https://bugs.kde.org/show_bug.cgi?id=329994
Repository: ki18n
Description
---
Kubuntu packagers had problems running make test because the ktranscript
test relied on the ktranscript plugin being installed to run. This RR avoids
this by loading the plugin from the build dir.
Diffs
-
autotests/ktranscripttest.cpp db8139a
Diff: https://git.reviewboard.kde.org/r/115049/diff/
Testing
---
Reverted install dir to a before KF5 state, installed KJS, built ki18n
without installing, ran the test successfully.
Thanks,
Aurélien Gâteau
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115030: Install public headers for KJsEmbed
On Jan. 16, 2014, 3:48 p.m., Alex Merry wrote: Still not ideal, but -- in the absence of an enthusiastic maintainer -- that's unlikely to change any time soon. At least this makes it usable, and the names shouldn't clash. :) that's the spirit! Anyway, without installing headers, the library is useless. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115030/#review47516 --- On Jan. 16, 2014, 2:33 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115030/ --- (Updated Jan. 16, 2014, 2:33 p.m.) Review request for KDE Frameworks. Repository: kjsembed Description --- KJsEmbed library is useless without headers installed. This patch installs them. Diffs - CMakeLists.txt fc1731a src/kjsembed/CMakeLists.txt b4d3f5f src/kjsembed/kjseglobal.h c2b44dc src/kjsembed/pointer.h e25a5e4 Diff: https://git.reviewboard.kde.org/r/115030/diff/ Testing --- I ported the share dataengine to use it (it was using the now defunct Kross KJS support). https://git.reviewboard.kde.org/r/115027/ Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114699: Use enums instead of ints in method types
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114699/#review47533 --- Ship it! Seems like a good idea :) Potentially breaks the source compatibility, but then it improves the API considerably. - Aleix Pol Gonzalez On Jan. 16, 2014, 1:28 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114699/ --- (Updated Jan. 16, 2014, 1:28 p.m.) Review request for KDE Frameworks and David Faure. Repository: kmediaplayer Description --- Use enums instead of ints in method types. This adds to the porting effort a little (but not in many places), but gives more type-safety. Diffs - autotests/playertest.cpp 638e86fecf7eb418c802b1f46d04e864d8e161f6 autotests/testplayer.h 285da38fdf886ec8710e3a0d595ca20777426f86 autotests/viewtest.cpp 337ac3eef735dab7860bb6577c72fa33d3597068 src/kmediaplayer/player.h 57935bae537dba355b4c0cf2306dd6c993762661 src/kmediaplayer/player.cpp c0be47666094a12abc160538798efafe1e3e1fb0 src/kmediaplayer/view.h c3af06b51202b29f93ca0e3509ad41b8bbb91b0a src/kmediaplayer/view.cpp 51e4c4ba5b16a4d7b90e57f78281fedb524eca54 Diff: https://git.reviewboard.kde.org/r/114699/diff/ Testing --- Builds, tests run. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115030: Install public headers for KJsEmbed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115030/ --- (Updated Jan. 16, 2014, 6:30 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kjsembed Description --- KJsEmbed library is useless without headers installed. This patch installs them. Diffs - CMakeLists.txt fc1731a src/kjsembed/CMakeLists.txt b4d3f5f src/kjsembed/kjseglobal.h c2b44dc src/kjsembed/pointer.h e25a5e4 Diff: https://git.reviewboard.kde.org/r/115030/diff/ Testing --- I ported the share dataengine to use it (it was using the now defunct Kross KJS support). https://git.reviewboard.kde.org/r/115027/ Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115077: Rename Macros in KF5DocTools to KDE5_
On Jan. 17, 2014, 6:51 p.m., Alex Merry wrote: KF5DocToolsMacros.cmake, lines 166-172 https://git.reviewboard.kde.org/r/115077/diff/1/?file=234284#file234284line166 These should *not* be renamed, as they are compatibility macros. However, they should probably be moved to kde4support. I wouldn't rename it indeed. Moving it to KDE4Support can be good indeed, although it kind of defeats the purpose, as you'll need to actually depend on KDE4Support to get the warning. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/#review47592 --- On Jan. 17, 2014, 3:14 p.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/ --- (Updated Jan. 17, 2014, 3:14 p.m.) Review request for Documentation and KDE Frameworks. Repository: kdoctools Description --- Part of the overall task of removing mentions of KDE4 from the code. Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115077/diff/ Testing --- Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115198: Fix KDE4Support compilation
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115198/ --- Review request for KDE Frameworks and David Faure. Repository: kde4support Description --- KCrash::setApplicationName and KCrash::setApplicationPath disappeared from the KCrash module 179de6d16fb9be580dbb7b15e0a56fcb990c7516, so I guess that a good fix is just stop using it. I'm unsure what's the best way though. Diffs - src/kdeui/kapplication.cpp 5a7f4c8 Diff: https://git.reviewboard.kde.org/r/115198/diff/ Testing --- Builds. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115202: Allow building KConfigWidgets on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115202/#review47947 --- CMakeLists.txt https://git.reviewboard.kde.org/r/115202/#comment33948 What's the problem with finding documentation on windows? - Aleix Pol Gonzalez On Jan. 21, 2014, 10:26 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115202/ --- (Updated Jan. 21, 2014, 10:26 p.m.) Review request for KDE Frameworks. Repository: kconfigwidgets Description --- Make KDocTools an optional dependency Fix compilation on MSVC Not sure whether this is a bug in MSVC or their STL or even Qt (unlikely) Diffs - CMakeLists.txt b5b42fb207d68e44bc0fe58763a39574d81c3e9d src/kcolorschememanager.cpp 2e44f88d4b613a263047899b102da53541139171 Diff: https://git.reviewboard.kde.org/r/115202/diff/ Testing --- Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115199: Fix detection of shared-mime-info on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115199/#review47959 --- Ship it! Looks good to me. - Aleix Pol Gonzalez On Jan. 21, 2014, 11:30 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115199/ --- (Updated Jan. 21, 2014, 11:30 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Fix detection of shared-mime-info on Windows For some reason exec_program would fail, it works with execute_process Diffs - cmake/FindSharedMimeInfo.cmake 4427c733b3c4e6fb969ee03e4405fb2a12a26232 Diff: https://git.reviewboard.kde.org/r/115199/diff/ Testing --- Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115207: Improve integration QCommandLineParser - KAboutData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115207/ --- Review request for KDE Frameworks. Repository: kcoreaddons Description --- Let the KAboutData set information to QApplication. This way we don't have to duplicate information by passing it to the KAboutData _and_ the QApplication. Diffs - src/lib/kaboutdata.h c9e src/lib/kaboutdata.cpp f24006b Diff: https://git.reviewboard.kde.org/r/115207/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115213: Remove KDE4_CREATE_HTML_HANDBOOK
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115213/#review47995 --- Well, but frameworks are not only for frameworks. They're all ported because I ported them. OTOH, there will be non-ported applications, that's why we provide this warning. - Aleix Pol Gonzalez On Jan. 22, 2014, 7:01 a.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115213/ --- (Updated Jan. 22, 2014, 7:01 a.m.) Review request for KDE Frameworks and Luigi Toscano. Repository: kdoctools Description --- As discussed in Review Request 115077 Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115213/diff/ Testing --- Searched all CMakeLists.txt files of frameworks for that macro, found nothing. Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115211: Mark target created by ecm_add_test as non GUI by default
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115211/#review47996 --- Ship it! Ship It! - Aleix Pol Gonzalez On Jan. 22, 2014, 1:28 a.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115211/ --- (Updated Jan. 22, 2014, 1:28 a.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Mark target created by ecm_add_test as non GUI by default This behaviour can be overriden by passing the GUI flag to the command Diffs - modules/ECMAddTests.cmake ff97d764d58c781d6c37dd08c8cb175ce500962e Diff: https://git.reviewboard.kde.org/r/115211/diff/ Testing --- Oketeta unit tests wouldn't build on windows before this commit, now they do Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115198: Fix KDE4Support compilation
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115198/ --- (Updated Jan. 22, 2014, 11:40 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks and David Faure. Repository: kde4support Description --- KCrash::setApplicationName and KCrash::setApplicationPath disappeared from the KCrash module 179de6d16fb9be580dbb7b15e0a56fcb990c7516, so I guess that a good fix is just stop using it. I'm unsure what's the best way though. Diffs - src/kdeui/kapplication.cpp 5a7f4c8 Diff: https://git.reviewboard.kde.org/r/115198/diff/ Testing --- Builds. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115213: Remove KDE4_CREATE_HTML_HANDBOOK
On Jan. 22, 2014, 11:32 a.m., Aleix Pol Gonzalez wrote: Well, but frameworks are not only for frameworks. They're all ported because I ported them. OTOH, there will be non-ported applications, that's why we provide this warning. Luigi Toscano wrote: You ported KDE4_CREATE_HANDBOOK, which is widely used in kdelibs4-based code: http://lxr.kde.org/search?filestring=string=KDE4_CREATE_HANDBOOK On the other side, KDE4_CREATE_HTML_HANDBOOK has never been used in a stable version (not in 4 at least), please check the dates: http://mail.kde.org/pipermail/kde-buildsystem/2007-January/003643.html http://quickgit.kde.org/?p=kdelibs.gita=commith=61dd00ab3211bb7b76a94344ed8d577a6d194cf1 http://quickgit.kde.org/?p=kdelibs.gita=commith=82f7b6ba0f8be7d314ac780b9a873e98b6be39b2 Luigi Toscano wrote: Also: http://lxr.kde.org/search?filestring=string=KDE4_CREATE_HTML_HANDBOOK Oh ok, sorry about the noise then. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115213/#review47995 --- On Jan. 22, 2014, 7:01 a.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115213/ --- (Updated Jan. 22, 2014, 7:01 a.m.) Review request for KDE Frameworks and Luigi Toscano. Repository: kdoctools Description --- As discussed in Review Request 115077 Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115213/diff/ Testing --- Searched all CMakeLists.txt files of frameworks for that macro, found nothing. Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115213: Remove KDE4_CREATE_HTML_HANDBOOK
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115213/#review48003 --- Ship it! Ship It! - Aleix Pol Gonzalez On Jan. 22, 2014, 7:01 a.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115213/ --- (Updated Jan. 22, 2014, 7:01 a.m.) Review request for KDE Frameworks and Luigi Toscano. Repository: kdoctools Description --- As discussed in Review Request 115077 Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115213/diff/ Testing --- Searched all CMakeLists.txt files of frameworks for that macro, found nothing. Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115211: Mark target created by ecm_add_test as non GUI by default
On Jan. 22, 2014, 12:08 p.m., Alex Merry wrote: This seems sensible to me; however, I do wonder if ECM should also provide an ecm_mark_gui_executable function as well (I'm thinking of the case where most of the tests should be non-gui, but a handful want to display widgets). Well, we can't change the default, can we? - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115211/#review48009 --- On Jan. 22, 2014, 1:28 a.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115211/ --- (Updated Jan. 22, 2014, 1:28 a.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Mark target created by ecm_add_test as non GUI by default This behaviour can be overriden by passing the GUI flag to the command Diffs - modules/ECMAddTests.cmake ff97d764d58c781d6c37dd08c8cb175ce500962e Diff: https://git.reviewboard.kde.org/r/115211/diff/ Testing --- Oketeta unit tests wouldn't build on windows before this commit, now they do Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115211: Mark target created by ecm_add_test as non GUI by default
On Jan. 22, 2014, 12:08 p.m., Alex Merry wrote: This seems sensible to me; however, I do wonder if ECM should also provide an ecm_mark_gui_executable function as well (I'm thinking of the case where most of the tests should be non-gui, but a handful want to display widgets). Aleix Pol Gonzalez wrote: Well, we can't change the default, can we? Alex Merry wrote: I don't understand what you mean. If we have a /mark as non gui/ function is because executables are /gui executables/ by default. Having an ecm_mark_gui_executable() would make this weird I'd say. (TBH, it seems to me cmake shouldn't know about that at all, but I take it as just a limitation on Windows (and Android)) - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115211/#review48009 --- On Jan. 22, 2014, 1:28 a.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115211/ --- (Updated Jan. 22, 2014, 1:28 a.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Mark target created by ecm_add_test as non GUI by default This behaviour can be overriden by passing the GUI flag to the command Diffs - modules/ECMAddTests.cmake ff97d764d58c781d6c37dd08c8cb175ce500962e Diff: https://git.reviewboard.kde.org/r/115211/diff/ Testing --- Oketeta unit tests wouldn't build on windows before this commit, now they do Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115238: Bug in KDEPlatformFileDialogHelper(?) causes selectFile not to work in QFileDialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115238/#review48086 --- Ship it! tests/qfiledialogtest.cpp https://git.reviewboard.kde.org/r/115238/#comment34038 no need for this debug. Thanks for improving the test, will have to look into the implementation. :) - Aleix Pol Gonzalez On Jan. 22, 2014, 10:26 p.m., Gregor Mi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115238/ --- (Updated Jan. 22, 2014, 10:26 p.m.) Review request for KDE Frameworks and David Faure. Repository: frameworkintegration Description --- Bug in KDEPlatformFileDialogHelper (or related code) as described below. The patch extends the qfiledialogtest so that the following command is possible: Run ./qfiledialogtest --acceptMode save --selectFile ~/moo.png Expected: - a Save File Dialog opens and the text field is filled with moo.png and the directory switches to the given path - The OK button is active so that the user can click it and the dialog closes Actual: - a Save File Dialog opens but the text field is NOT filled and the location does not change - The OK button is not active - Workaround: click on an existing file and the OK button becomes active Diffs - tests/qfiledialogtest.cpp 3ef79a2b6787bd42dfa32a6c641589755436 Diff: https://git.reviewboard.kde.org/r/115238/diff/ Testing --- Thanks, Gregor Mi ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115028: Allow the building of deprecated code to be disabled
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115028/#review48324 --- Shouldn't we maybe just remove these? Especially considerign they already are deprecated in kdelibs 4. I don't really like disabling compilation of deprecated symbols, especially in this case we're not winning that much. - Aleix Pol Gonzalez On Jan. 15, 2014, 1:56 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115028/ --- (Updated Jan. 15, 2014, 1:56 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- This is mostly an example of how we could improve the deprecation handling. There are two parts: preventing deprecation warnings when building the library itself (see http://build.kde.org/view/Frameworks/job/kwidgetsaddons_master_qt5/11/warnings17Result/NORMAL/package.-1402078525/ for examples) and allowing the framework to be built with no deprecated code. We possibly want to export the fact that the framework was built without deprecated code via the CMake config file, so that downstream stuff (like kde4support) can check for it and complain if necessary. Allow the building of deprecated code to be disabled This adds a CMake option to enable or disable the building of deprected code. It just changes the kcoreaddons_export.h file. Part of this change is to use KCOREADDONS_NO_DEPRECATED instead of KDE_NO_DEPRECATED. Disable deprecation macro when building the library itself This prevents spurious compiler warnings (particularly when slots are deprecated). Diffs - src/lib/CMakeLists.txt 8cc71f34e671962f2d7268b3db0d50e6750c26a2 src/lib/util/kuser.h 2b6e6ed92bc1465945f36f2fde821f36fa51585f src/lib/util/kuser_unix.cpp 8a3a39d379ca863b4906bb01228c5e01a5b955b0 src/lib/util/kuser_win.cpp 6a6cbb1751bd569d8684f8e11add1ef304c0a94d Diff: https://git.reviewboard.kde.org/r/115028/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114997: Improve KAuth README.md
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114997/#review48325 --- README.md https://git.reviewboard.kde.org/r/114997/#comment34187 Need to use? link against? Also the (or similar) looks unsure. I would say: If you are using cmake, you can find KAuth by using: find_package(KF5Auth NO_MODULE) or finding KF5 with the Auth component, from your CMake scripts. - Aleix Pol Gonzalez On Jan. 26, 2014, 5:39 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114997/ --- (Updated Jan. 26, 2014, 5:39 p.m.) Review request for KDE Frameworks and Dario Freddi. Repository: kauth Description --- Improve KAuth README.md Diffs - README.md a8a011a147d2dcc0fb5db39e263412005a86def4 Diff: https://git.reviewboard.kde.org/r/114997/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115148: Add KWINDOWSYSTEM_ namespace to HAVE_FOO defines
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115148/#review48326 --- Ship it! Makes sense to me. - Aleix Pol Gonzalez On Jan. 26, 2014, 5:36 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115148/ --- (Updated Jan. 26, 2014, 5:36 p.m.) Review request for KDE Frameworks. Repository: kwindowsystem Description --- Add KWINDOWSYSTEM_ namespace to HAVE_FOO defines config-kwindowsystem.h is installed and included by public headers, so it should not define things as generic as HAVE_X11, which are likely to also be defined by users of the framework. Diffs - src/netwm.cpp 266afccaf111e6707493b18dad1d9c03dde1d912 src/netwm.h aee6cea5e3b65cf6252b50515e4920cb9c96f240 src/kxerrorhandler.cpp 612e62a345ec9c8503bab9fb12afa08426974828 src/kxerrorhandler_p.h ad670fec6ff35e1c9df81b91517c02e6e0893ad1 src/kxmessages.h 5d57e06d702afb5ceaed05de97ca40116b7f3aa3 src/kxmessages.cpp b409ef2114c18190188ba5503d2f357bd9336e76 src/kxutils.cpp e47ba68c08a14597e49fd33901b0a7c079924112 src/kxutils_p.h 1e229801aac929bc596685dbfa5260d55d0f5298 src/kusertimestamp.cpp de8ca61e7e9dd0ae9492ccf61883560d80501e2b src/kstartupinfo.cpp 5dbf47cb666fbed17c943491efe93e17f27d581e src/kkeyserver.h 8869fe7331e98846183ab37814b1d95e75ab9647 src/config-kwindowsystem.h.cmake 58949d87f4254531ba57de86b6303cba05053222 src/CMakeLists.txt b453af11a46615ecd94911f0d2f86922087bde0e CMakeLists.txt 2479237e574e86c9094b557dfa146c85b7b19b65 src/kwindoweffects.cpp 91ef52f149a781df9308bb1587629dbaaf571b8e src/kwindoweffects_p.h 266b5f3e7f24bdca6ce467138ee067335225da78 src/kwindowsystem.h 5bcfd062dcca42d282b70d0683d4ee1da88cc814 src/kwindowsystem_x11.cpp 8634240cabc708a608277b34f21c41cee295e48a Diff: https://git.reviewboard.kde.org/r/115148/diff/ Testing --- Clean configure-build-test-install on a system with X11. KWindowSystem already failed to build on a non-windows, non-mac system when X11 is not found (one of the tests fails to link if you comment out the find_package(X11) calls, as KWindowSystem::activeWindow() is never defined). Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115316: Add demo for KRecentFileList
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115316/#review48327 --- It's a test, not a demo. If you want, it's for demonstrating the developer that he did it right, but I wouldn't see it as documentation. I would rename it to KRecentFilesActionTest - Aleix Pol Gonzalez On Jan. 25, 2014, 10:56 p.m., Gregor Mi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115316/ --- (Updated Jan. 25, 2014, 10:56 p.m.) Review request for KDE Frameworks. Repository: kconfigwidgets Description --- This is a demo test for KRecentFileList (in combination with KSharedConfig). The patch also contains a documentation update for krecentfilesaction.h/loadEntries() saying that Local file entries that do not exist anymore are not restored.. As a side note: Does it makes sense to optionally disable the automated removal of non-existing recent files? Or have the possibility to return the files that were automatically removed? Diffs - src/krecentfilesaction.h 38d1b5e3455d081304b064d13bccfc2164d12a14 tests/CMakeLists.txt 617a55944b2c91c9b09125ad92d070d3cd9a6551 tests/krecentfilesactiondemo.h PRE-CREATION tests/krecentfilesactiondemo.cpp PRE-CREATION tests/krecentfilesactiondemo.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115316/diff/ Testing --- Thanks, Gregor Mi ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115207: Improve integration QCommandLineParser - KAboutData
On Jan. 27, 2014, 7:45 a.m., Kevin Ottens wrote: src/lib/kaboutdata.cpp, line 941 https://git.reviewboard.kde.org/r/115207/diff/1/?file=235099#file235099line941 Not really my type of thing. It's acting on an object behind our back without knowing... what happens to code where setApplication* was called before? Information would be lost and it's not obvious to the user. Looks dangerous to me. If we want to factorize the qApp call, and I see the need for that indeed, then that block should be provided by a separate method (setupApplication(QCoreApplication *)?). I agree, I only did it this way to avoid the required set up. An alternative would be to make KAboutData to pass the information to Q*Application when it's initialized (and if it's an application KAboutData...). - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115207/#review48347 --- On Jan. 21, 2014, 11:36 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115207/ --- (Updated Jan. 21, 2014, 11:36 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Let the KAboutData set information to QApplication. This way we don't have to duplicate information by passing it to the KAboutData _and_ the QApplication. Diffs - src/lib/kaboutdata.h c9e src/lib/kaboutdata.cpp f24006b Diff: https://git.reviewboard.kde.org/r/115207/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115361: use renamed kmailservice5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115361/#review48489 --- Ship it! Ship It! - Aleix Pol Gonzalez On Jan. 28, 2014, 4:25 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115361/ --- (Updated Jan. 28, 2014, 4:25 p.m.) Review request for KDE Frameworks and Hrvoje Senjan. Repository: kservice Description --- fix test for renamed kmailservice5 proposed in https://git.reviewboard.kde.org/r/115359/ Diffs - autotests/kservicetest.cpp 89eb0ae Diff: https://git.reviewboard.kde.org/r/115361/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115362: Do not explicitly link against libc
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115362/#review48490 --- Ship it! I tried it here, doesn't seem to break. Also it's really ugly to have -lc so +1 from me. - Aleix Pol Gonzalez On Jan. 28, 2014, 4:41 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115362/ --- (Updated Jan. 28, 2014, 4:41 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Do not explicitly link against libc This is entirely unnecessary. Diffs - kde-modules/KDECompilerSettings.cmake 3419cb1266c63f9cdec0501ae340aabe7c0b6096 Diff: https://git.reviewboard.kde.org/r/115362/diff/ Testing --- KCoreAddons and KDE4Support still configure and compile (GCC 4.8.2 20131219; Linux with glibc 2.18). Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115382: Remove unused dependencies
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115382/#review48548 --- Ship it! Cool :) - Aleix Pol Gonzalez On Jan. 29, 2014, 3:33 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115382/ --- (Updated Jan. 29, 2014, 3:33 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- QtGui is not used anywhere, and QtWidgets are only required for tests. Diffs - src/kcrash.cpp b8e13a92c822d0bec47280941e7d5dadca5bfb38 src/CMakeLists.txt 69dd376a0f08909d77b70c492fc60a5ab5220317 CMakeLists.txt 0ce523b9a5355813285c508fcff8f4c6b80405ba KF5CrashConfig.cmake.in f1e6ecfee32f4c54d467f9a08976472c16fe6823 autotests/CMakeLists.txt b01a5753adb0a3097b315570231edfbff30f89d7 Diff: https://git.reviewboard.kde.org/r/115382/diff/ Testing --- Builds. I didn't find any source references to the two dependencies. qwindowdefs.h from QtWidgets was identified as unused through static analysis tool. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115411: Remove obsolete CMake code from pre-splitting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115411/#review48682 --- Ship it! I think we should agree on doing this for all frameworks, actually. All this checking if it's being built out of kdelibs doesn't make sense anymore. - Aleix Pol Gonzalez On Jan. 31, 2014, 12:03 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115411/ --- (Updated Jan. 31, 2014, 12:03 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- Remove obsolete CMake code from pre-splitting Diffs - CMakeLists.txt bf678c41b78ab984a5b30a278a21ceaeddeeccff Diff: https://git.reviewboard.kde.org/r/115411/diff/ Testing --- Builds fine. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115421: Clean up the CMakeLists.txt files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115421/#review48815 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 1, 2014, 1:27 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115421/ --- (Updated Feb. 1, 2014, 1:27 p.m.) Review request for KDE Frameworks. Repository: frameworkintegration Description --- Clean up the CMakeLists.txt files - removed unnecessary cruft from the top-level CMakeLists.txt - KStyle can once again be built standalone - general formatting improvements Diffs - CMakeLists.txt 4135a6fcd2f645b26a2f1f7ce374c677afd5ca16 src/integrationplugin/CMakeLists.txt fa1e63da3436f18ed997aa96296b60e2fff3e822 src/kstyle/CMakeLists.txt 89d8e36fcd0305090c67cecf8ca48ab28d28a4d6 src/platformtheme/CMakeLists.txt 141c788b46cb37bf4e6410c46523fa07d918c382 Diff: https://git.reviewboard.kde.org/r/115421/diff/ Testing --- Both frameworkintegration and the src/kstyle subdir confgure, build and install. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115477: Remove commands that just set variables to their defaults
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115477/#review48995 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 4, 2014, 4:36 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115477/ --- (Updated Feb. 4, 2014, 4:36 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Remove commands that just set variables to their defaults Diffs - kde-modules/KDECMakeSettings.cmake 24593d4af57e982b8c772b1931b805872d518d21 Diff: https://git.reviewboard.kde.org/r/115477/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115472: fix test for icon path
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115472/#review48996 --- autotests/kdeplatformtheme_unittest.cpp https://git.reviewboard.kde.org/r/115472/#comment34595 You probably want endsWith(/.icons). - Aleix Pol Gonzalez On Feb. 4, 2014, 4:30 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115472/ --- (Updated Feb. 4, 2014, 4:30 p.m.) Review request for KDE Frameworks and Àlex Fiestas. Repository: frameworkintegration Description --- QPlatformTheme::IconThemeSearchPaths is ~/.icons in this Kubuntu install so fix test to allow for this Diffs - autotests/kdeplatformtheme_unittest.cpp 2f9943f Diff: https://git.reviewboard.kde.org/r/115472/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115472: fix test for icon path
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115472/#review49019 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 5, 2014, 11:32 a.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115472/ --- (Updated Feb. 5, 2014, 11:32 a.m.) Review request for KDE Frameworks and Àlex Fiestas. Repository: frameworkintegration Description --- QPlatformTheme::IconThemeSearchPaths is ~/.icons in this Kubuntu install so fix test to allow for this Diffs - autotests/kdeplatformtheme_unittest.cpp 2f9943f Diff: https://git.reviewboard.kde.org/r/115472/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115525: Deprecate slots in KCompletion and convert into slots the methods they call
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115525/#review49161 --- Ship it! Looks good to me - Aleix Pol Gonzalez On Feb. 6, 2014, 11:09 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115525/ --- (Updated Feb. 6, 2014, 11:09 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Deprecate three slots (slotFoo()) that the only thing they do is call another method (foo()) Diffs - src/kcompletion.h 2104f1b Diff: https://git.reviewboard.kde.org/r/115525/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115530: Find Qt5::X11Extras only if X11 is found
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115530/#review49177 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 7, 2014, 8:46 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115530/ --- (Updated Feb. 7, 2014, 8:46 a.m.) Review request for KDE Frameworks. Repository: kwindowsystem Description --- Find Qt5::X11Extras only if X11 is found No need to have a required dependency on X11Extras if we don't build for X11. Diffs - CMakeLists.txt 6cebf0cb22f5c0192e0423c4084f12b9dda75151 Diff: https://git.reviewboard.kde.org/r/115530/diff/ Testing --- compiles with and without X11 Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115616: Add platform to qt options in KCmdLineArgs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115616/#review49439 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 10, 2014, 9:14 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115616/ --- (Updated Feb. 10, 2014, 9:14 a.m.) Review request for KDE Frameworks. Repository: kde4support Description --- Add platform to qt options in KCmdLineArgs It's needed to start KApplications on Wayland. Diffs - src/kdecore/kcmdlineargs.cpp 2b5ed04c03a6a66cf776dfc09d8c0ca49ac432ae Diff: https://git.reviewboard.kde.org/r/115616/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115629: Port DrKonqi to QCommandLineParser and QCommandLineOption
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115629/#review49455 --- drkonqi/main.cpp https://git.reviewboard.kde.org/r/115629/#comment34891 You can instantiate QApplication in the stack, instead of calling new+delete. Also you probably want to create it in the beginning of the main function body. - Aleix Pol Gonzalez On Feb. 10, 2014, 4:06 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115629/ --- (Updated Feb. 10, 2014, 4:06 p.m.) Review request for KDE Frameworks and Jekyll Wu. Repository: kde-runtime Description --- The parsed command line values are kept in the DrKonqi singleton which replaces the static access to the KCmdLineArgs. Diffs - drkonqi/drkonqi.h 95e64dc drkonqi/drkonqi.cpp ccb1c42 drkonqi/drkonqibackends.cpp 064d07d drkonqi/drkonqidialog.cpp 3fc1549 drkonqi/main.cpp 1337dbe Diff: https://git.reviewboard.kde.org/r/115629/diff/ Testing --- crashed one app, DrKonqi opened and all information seemed reasonable. Though I haven't tested all options as I also don't know all of their meaning. Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115629: Port DrKonqi to QCommandLineParser and QCommandLineOption
On Feb. 10, 2014, 4:14 p.m., Aleix Pol Gonzalez wrote: drkonqi/main.cpp, line 74 https://git.reviewboard.kde.org/r/115629/diff/1/?file=243089#file243089line74 You can instantiate QApplication in the stack, instead of calling new+delete. Also you probably want to create it in the beginning of the main function body. Martin Gräßlin wrote: I'm fine with changing it, but I'd like the opinion of one of the people who are more familiar with the code base. It used to have the QApplication pointer and I don't know why. I assume it's because it used to either create a QApplication or a KApplication. If that's the only reason I'm happy to change the code, otherwise I would keep it with the pointer variant. Fair enough, other than that +1. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115629/#review49455 --- On Feb. 10, 2014, 4:06 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115629/ --- (Updated Feb. 10, 2014, 4:06 p.m.) Review request for KDE Frameworks and Jekyll Wu. Repository: kde-runtime Description --- The parsed command line values are kept in the DrKonqi singleton which replaces the static access to the KCmdLineArgs. Diffs - drkonqi/drkonqi.h 95e64dc drkonqi/drkonqi.cpp ccb1c42 drkonqi/drkonqibackends.cpp 064d07d drkonqi/drkonqidialog.cpp 3fc1549 drkonqi/main.cpp 1337dbe Diff: https://git.reviewboard.kde.org/r/115629/diff/ Testing --- crashed one app, DrKonqi opened and all information seemed reasonable. Though I haven't tested all options as I also don't know all of their meaning. Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115681: kquitapp - kquitapp5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115681/#review49609 --- Ship it! - Aleix Pol Gonzalez On Feb. 11, 2014, 9:04 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115681/ --- (Updated Feb. 11, 2014, 9:04 p.m.) Review request for KDE Frameworks and David Faure. Repository: kdbusaddons Description --- otherwise collides with kde-runtime(4)... Diffs - src/tools/kquitapp/CMakeLists.txt d095617 Diff: https://git.reviewboard.kde.org/r/115681/diff/ Testing --- builds Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115684: Generate local forwarding headers under a local subdir, to fix clash on Mac OS X.
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115684/#review49615
---
Ship it!
Makes sense to me, I most certainly didn't consider it as a problem.
This could break compilation on some projects other than KParts, will you be
able to try the rest of the modules?
Thanks for figuring it out!
- Aleix Pol Gonzalez
On Feb. 11, 2014, 10:15 p.m., David Faure wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115684/
---
(Updated Feb. 11, 2014, 10:15 p.m.)
Review request for Build System, Extra Cmake Modules, KDE Frameworks, and
Harald Fernengel.
Repository: extra-cmake-modules
Description
---
Generate local forwarding headers under a local subdir, to fix clash on Mac
OS X.
This is intended to replace RR 115541.
With case-insensitive filesystems, creating KParts and kparts subdirs
in the same parent was obviously a bad idea, especially since we then
make a copy of KParts and don't expect the contents of kparts to tag
along.
Solved by making that KParts (installed) and local/kparts (not installed).
Downside: the modules that use this PREFIX feature need a change like this:
-target_include_directories(KF5Parts PUBLIC
$BUILD_INTERFACE:${KParts_BINARY_DIR})
+target_include_directories(KF5Parts PUBLIC
$BUILD_INTERFACE:${KParts_BINARY_DIR};${CMAKE_CURRENT_BINARY_DIR}/local)
Easily scripted though:
perl -pi -e 's//\;\${CMAKE_CURRENT_BINARY_DIR}\/local/ if
(/target_include_directories/ /PUBLIC/)' `grep -rwl PREFIX .`
Diffs
-
modules/ECMGenerateHeaders.cmake e98a22e91151d23d7c798ff22a33097ec2a59d10
Diff: https://git.reviewboard.kde.org/r/115684/diff/
Testing
---
Applied it, ran the perl script, and a full build-from-scratch worked.
Not tested on a Mac, though :)
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115683: Install app desktop files to share/applications, not in a kde5 subdir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115683/#review49616 --- Ship it! I would say it makes sense, I don't know about any benefits of name-spacing the desktop files besides feeling different and special. :) - Aleix Pol Gonzalez On Feb. 11, 2014, 9:56 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115683/ --- (Updated Feb. 11, 2014, 9:56 p.m.) Review request for Build System, Extra Cmake Modules, KDE Frameworks, and David Faure. Repository: extra-cmake-modules Description --- Install app desktop files to share/applications, not in a kde5 subdir Given that binaries are all installed in PREFIX/bin, and have to avoid clashes, doing the same for desktop files is no great issue, and installing into a subdirectory of applications/ just complicates matters for client code that needs to refer to the desktop file (is it kde5-foo[.desktop], kde5/foo[.desktop] or just foo[.desktop]?). Diffs - kde-modules/KDEInstallDirs.cmake 46e15c17d488d56f146aba0c2d420f74a22b9152 Diff: https://git.reviewboard.kde.org/r/115683/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115695: Rework KNotification to work without KNotify daemon
On Feb. 19, 2014, 10:06 a.m., Aleix Pol Gonzalez wrote: src/knotificationmanager.cpp, line 180 https://git.reviewboard.kde.org/r/115695/diff/3/?file=243841#file243841line180 ? Martin Klapetek wrote: Yet once again the description xD - it's full of ... FIXMEs to indicate what needs doing Still you shouldn't ship such comments, so it's probably why nobody checked the ship it thing. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115695/#review50197 --- On Feb. 13, 2014, 10:14 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115695/ --- (Updated Feb. 13, 2014, 10:14 a.m.) Review request for kde-workspace, KDE Frameworks, Plasma, and Sune Vuorela. Repository: knotifications Description --- This patch merges KNotify daemon into KNotificationManager to have less daemons running and less dbus traffic. The patch is not yet finished (and for now it's full of QDebugs, that will all be removed and FIXMEs to indicate what needs doing), but as the Alpha2 is quite soon, I'd like to start the general review asap so some more changes can be done if needed. Now it's KNotificationManager that handles the KNotifyPlugin-s and hands them the notification directly. KNotifyConfig was repurposed a bit, now it serves mostly just as the .notifyrc wrapper, all the rest is reused from the KNotification object. There are some changes in the KNotification API - id() and appName() are now exposed to public and slotReceivedId(int) is now also public so that KNotificationManager can directly give it an id. I'd like to rename this and make it a non-slot. It's not the DBus/Galago server ID anymore, that is handled in NotifyByPopup which is responsible for communicating with the galago server (all the methods there were renamed to actually have *galago* in the name so it's clear), therefore the mapping of DBus/Galago Server ids is managed only there as it is actually only needed here. KNotitification::id() is assigned by the KNotificationManager and it's a simple increasing counter. The UI/Plasmoid changes will come next - basically the plan is to put only the Persistent notifications in the notifications history. Diffs - src/knotifyconfig.h PRE-CREATION src/knotifyconfig.cpp PRE-CREATION src/knotifyplugin.h PRE-CREATION src/knotifyplugin.cpp PRE-CREATION src/notifybypopup.h PRE-CREATION src/notifybypopup.cpp PRE-CREATION src/notifybypopupgrowl.h PRE-CREATION src/notifybypopupgrowl.cpp PRE-CREATION CMakeLists.txt 63ebf71 src/CMakeLists.txt a81b913 src/knotification.h 00554ba src/knotification.cpp 5d7405b src/knotificationmanager.cpp a4d0dfa src/knotificationmanager_p.h 81d962d Diff: https://git.reviewboard.kde.org/r/115695/diff/ Testing --- Works perfectly with both plasma notifications and kpassivepopup. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115634: Add kconfig_compiler autotest that checks whether signals are emitted
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115634/#review50388 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 20, 2014, 3:53 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115634/ --- (Updated Feb. 20, 2014, 3:53 p.m.) Review request for KDE Frameworks. Repository: kconfig Description --- Add kconfig_compiler autotest that checks whether signals are emitted Currently this works when using the setters, however when using setProperty() on the KCoreConfigSkeleton* (as done by KConfigDialog) no signals are emitted. Diffs - autotests/kconfig_compiler/signals_test_no_singleton_dpointer.kcfgc PRE-CREATION autotests/kconfig_compiler/signals_test_singleton.kcfgc PRE-CREATION autotests/kconfig_compiler/signals_test_singleton_dpointer.kcfgc PRE-CREATION autotests/kconfig_compiler/CMakeLists.txt a2ebb9453bacb2c7507bc9477b6753a34bbcd434 autotests/kconfig_compiler/kconfigcompiler_test_signals.cpp PRE-CREATION autotests/kconfig_compiler/signals_test.kcfg PRE-CREATION autotests/kconfig_compiler/signals_test_no_singleton.kcfgc PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115634/diff/ Testing --- Compiles, tests fail until https://git.reviewboard.kde.org/r/115635/ is applied, then they pass. Rather ugly code IMO, open for suggestions to improve it... Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115207: Improve integration QCommandLineParser - KAboutData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115207/ --- (Updated Feb. 21, 2014, 12:26 p.m.) Review request for KDE Frameworks. Changes --- Move the QApplication initialization to setApplicationData() instead of in setupCommandLine(). It makes more sense, the previous API looked a bit hacky. Repository: kcoreaddons Description --- Let the KAboutData set information to QApplication. This way we don't have to duplicate information by passing it to the KAboutData _and_ the QApplication. Diffs (updated) - src/lib/kaboutdata.h c9e src/lib/kaboutdata.cpp c347521 Diff: https://git.reviewboard.kde.org/r/115207/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115207: Improve integration QCommandLineParser - KAboutData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115207/ --- (Updated Feb. 21, 2014, 2:03 p.m.) Review request for KDE Frameworks. Changes --- Address issues pointed out by Alex Merry. Repository: kcoreaddons Description --- Let the KAboutData set information to QApplication. This way we don't have to duplicate information by passing it to the KAboutData _and_ the QApplication. Diffs (updated) - src/lib/kaboutdata.h c9e src/lib/kaboutdata.cpp c347521 Diff: https://git.reviewboard.kde.org/r/115207/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116462: Keep KSharedConfigPtr(kdeglobals) around in KHintSettings.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116462/#review51729 --- Ship it! Makes sense to me. - Aleix Pol Gonzalez On Feb. 27, 2014, 9:59 p.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116462/ --- (Updated Feb. 27, 2014, 9:59 p.m.) Review request for KDE Frameworks and Aleix Pol Gonzalez. Repository: frameworkintegration Description --- Keep KSharedConfigPtr(kdeglobals) around in KHintSettings. This makes KFontSettingsData end up using the same instance, rather than each of these two classes parsing kdeglobals on startup, in turn. Had to fix the unittest again, it doesn't use QStandardPaths' testMode, so it must ensure to set its special environment before the platformtheme is created. strace -e open kate | grep -v NOENT | grep kdeglobals | wc -l went from 5 to 4. Diffs - src/platformtheme/khintssettings.cpp c4de4badc6187d841af36e29e083ffd2ca475d82 src/platformtheme/khintssettings.h 8f353837884590bc6a8409df7273435824273f49 src/platformtheme/kfontsettingsdata.cpp 62990ce45c1a175c3b57710c8a38268d08908733 autotests/kfontsettingsdata_unittest.cpp 4ee33fea72905128112226248667499489b1c692 Diff: https://git.reviewboard.kde.org/r/116462/diff/ Testing --- as described: unittests in frameworkintegration + running kate. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116540: Add configuration for ReviewBoard
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116540/#review51731 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 2, 2014, 8:10 p.m., Cornelius Schumacher wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116540/ --- (Updated March 2, 2014, 8:10 p.m.) Review request for KDE Frameworks. Repository: krunner Description --- Add configuration for ReviewBoard Diffs - .reviewboardrc PRE-CREATION Diff: https://git.reviewboard.kde.org/r/116540/diff/ Testing --- Thanks, Cornelius Schumacher ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
