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
Re: Review Request 116539: Add README
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116539/#review51732 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 2, 2014, 8:08 p.m., Cornelius Schumacher wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116539/ --- (Updated March 2, 2014, 8:08 p.m.) Review request for KDE Frameworks. Repository: krunner Description --- Add README Diffs - README.md PRE-CREATION Diff: https://git.reviewboard.kde.org/r/116539/diff/ Testing --- Thanks, Cornelius Schumacher ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116573: Make KI18n a private dependency in KIO
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116573/ --- Review request for KDE Frameworks and David Faure. Repository: kio Description --- Don't include klocalizedstring.h from slavebase.h. It's odd and it was not specified by CMakeLists.txt (the alternative is to move KI18n to LINK_PUBLIC). Diffs - src/ioslaves/help/kio_help.cpp 38e09d5 src/ioslaves/http/http.cpp a0193a0 src/ioslaves/ftp/ftp.cpp 706c983 src/core/slavebase.h 86f1506 src/core/slavebase.cpp 1236ad5 src/ioslaves/file/file.cpp b8c6220 src/ioslaves/file/file_unix.cpp 96fd6af Diff: https://git.reviewboard.kde.org/r/116573/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 116103: Make KI18N a public dependency of KIO since public installed headers of KIO requires it
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116103/#review51844 --- Ok, I just realized this was being dealt with and I did a different patch: https://git.reviewboard.kde.org/r/116573/ I think that having UI strings on a header file is quite bad TBH, but since I see there's consensus I'll discard it. - Aleix Pol Gonzalez On March 3, 2014, 8:59 p.m., Matthieu Gallien wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116103/ --- (Updated March 3, 2014, 8:59 p.m.) Review request for KDE Frameworks. Repository: kio Description --- include/KF5/KIOCore/kio/slavebase.h is including headers from KI18N and is publically installed. Diffs - KF5KIOConfig.cmake.in 3a947b7 src/core/CMakeLists.txt d897e37 Diff: https://git.reviewboard.kde.org/r/116103/diff/ Testing --- Thanks, Matthieu Gallien ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116573: Make KI18n a private dependency in KIO
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116573/ --- (Updated March 4, 2014, 12:18 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Repository: kio Description --- Don't include klocalizedstring.h from slavebase.h. It's odd and it was not specified by CMakeLists.txt (the alternative is to move KI18n to LINK_PUBLIC). Diffs - src/ioslaves/help/kio_help.cpp 38e09d5 src/ioslaves/http/http.cpp a0193a0 src/ioslaves/ftp/ftp.cpp 706c983 src/core/slavebase.h 86f1506 src/core/slavebase.cpp 1236ad5 src/ioslaves/file/file.cpp b8c6220 src/ioslaves/file/file_unix.cpp 96fd6af Diff: https://git.reviewboard.kde.org/r/116573/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 116598: Check versions for individual components of Wayland
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116598/#review51912
---
Ship it!
Looks good to me.
- Aleix Pol Gonzalez
On March 4, 2014, 3:53 p.m., Aurélien Gâteau wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116598/
---
(Updated March 4, 2014, 3:53 p.m.)
Review request for Build System, Extra Cmake Modules, KDE Frameworks, and
Martin Gräßlin.
Repository: extra-cmake-modules
Description
---
First part of the diff makes sure find_package_handle_standard_args() gets a
version number to check against.
Second part ensures we get proper results from pkg-config even if not all
components are available. find_package(Wayland COMPONENTS Client Egl) was
failing for me because I have Client installed but not Egl, causing
pkg_check_modules() to not set any PKG_Wayland_${comp} variable.
Diffs
-
find-modules/FindWayland.cmake 21014fc
Diff: https://git.reviewboard.kde.org/r/116598/diff/
Testing
---
Together with a change for kde-workspace, it fixes the build of kde-workspace
on my system with wayland-client 1.1 and no wayland-egl.
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 116603: Expose the QDialogButtonBox in KPasswordDialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116603/ --- Review request for KDE Frameworks. Repository: kwidgetsaddons Description --- KPasswordDialog used to be a KDialog. There, users could interact with the buttons setup (like sudlg.cpp in kde-runtime/kdesu). The fact that it was re-based to QDialog, we lost the ability of editing the buttons at all. This change exposes the buttonBox(), so the user re-gains this feature, through QDialogButtonBox. Diffs - src/kpassworddialog.h 069e301 src/kpassworddialog.cpp cacf31a Diff: https://git.reviewboard.kde.org/r/116603/diff/ Testing --- Ported sudlg.cpp to it, they need it because it requires an Ignore button over there. 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 March 4, 2014, 11:11 p.m.) Review request for KDE Frameworks. Changes --- Address David's remarks. I'm a noob. . 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 116603: Expose the QDialogButtonBox in KPasswordDialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116603/ --- (Updated March 5, 2014, 11:49 a.m.) Review request for KDE Frameworks. Changes --- Add documentation, makes the method protected, just in case. Repository: kwidgetsaddons Description --- KPasswordDialog used to be a KDialog. There, users could interact with the buttons setup (like sudlg.cpp in kde-runtime/kdesu). The fact that it was re-based to QDialog, we lost the ability of editing the buttons at all. This change exposes the buttonBox(), so the user re-gains this feature, through QDialogButtonBox. Diffs (updated) - src/kpassworddialog.h 069e301 src/kpassworddialog.cpp cacf31a Diff: https://git.reviewboard.kde.org/r/116603/diff/ Testing --- Ported sudlg.cpp to it, they need it because it requires an Ignore button over there. 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 March 5, 2014, 11:55 a.m.) Status -- This change has been marked as submitted. 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 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 116603: Expose the QDialogButtonBox in KPasswordDialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116603/ --- (Updated March 5, 2014, 12:04 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kwidgetsaddons Description --- KPasswordDialog used to be a KDialog. There, users could interact with the buttons setup (like sudlg.cpp in kde-runtime/kdesu). The fact that it was re-based to QDialog, we lost the ability of editing the buttons at all. This change exposes the buttonBox(), so the user re-gains this feature, through QDialogButtonBox. Diffs - src/kpassworddialog.h 069e301 src/kpassworddialog.cpp cacf31a Diff: https://git.reviewboard.kde.org/r/116603/diff/ Testing --- Ported sudlg.cpp to it, they need it because it requires an Ignore button over there. 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 116628: Avoid multiple warnings caused by CMake Policy 0026
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116628/#review52236 --- Ship it! - Aleix Pol Gonzalez On March 6, 2014, 10:11 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116628/ --- (Updated March 6, 2014, 10:11 a.m.) Review request for Build System and KDE Frameworks. Repository: kde4support Description --- I don't think there is a way to make this code CMP0026-compliant. Since it is supposed to not be used in the long run, disable the policy temporarily should be OK. Diffs - cmake/modules/KDE4Macros.cmake 192094b Diff: https://git.reviewboard.kde.org/r/116628/diff/ Testing --- Rebuilt kde-workspace with the change. CMake output is easier to read now. 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 116681: Fix kio-help by adding a separator
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116681/#review52470 --- Ship it! Looks correct. Yes, the port from KStandardDirs is quite fragile :/. - Aleix Pol Gonzalez On March 9, 2014, 9:18 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116681/ --- (Updated March 9, 2014, 9:18 p.m.) Review request for KDE Frameworks. Repository: kio Description --- Looking at the code it seems that QStandardPaths::locateAll() does not add the final '/' as the previous kdelibs code. Without this fix kio-help does find the documentation. General remark: maybe other parts of code which use QStandardPaths should be checked. Diffs - src/ioslaves/help/kio_help.cpp 85b7975 Diff: https://git.reviewboard.kde.org/r/116681/diff/ Testing --- kioclient cat help:/kioslave/ftp (from frameworks branch of kde-runtime) now works. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/#review52481 --- Ship it! I would say it makes sense. I wonder why it's not here... http://community.kde.org/Frameworks/Epics/New_Runtime_Organization - Aleix Pol Gonzalez On March 10, 2014, 12:01 a.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 10, 2014, 12:01 a.m.) Review request for Documentation and KDE Frameworks. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
On March 10, 2014, 4:32 a.m., Aleix Pol Gonzalez wrote: I would say it makes sense. I wonder why it's not here... http://community.kde.org/Frameworks/Epics/New_Runtime_Organization PS: note I'm not the KIO maintainer, you might want to wait for his advise. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/#review52481 --- On March 10, 2014, 12:01 a.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 10, 2014, 12:01 a.m.) Review request for Documentation and KDE Frameworks. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116696: Fix kservice_desktop_to_json when using cmake 2.8
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116696/ --- Review request for KDE Frameworks. Repository: kservice Description --- execute_process won't translate the generation expression into a path, so it refuses to work. In this case using get_property(... LOCATION) it's fine because it's only used as an imported target. Diffs - KF5ServiceMacros.cmake fd835bd Diff: https://git.reviewboard.kde.org/r/116696/diff/ Testing --- Now I can build frameworks that use kservice_desktop_to_json. 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 116696: Fix kservice_desktop_to_json when using cmake 2.8
On March 10, 2014, 2:59 p.m., Aurélien Gâteau wrote: Strange, it works fine here. Which version of CMake are you using and which repository is failing? Aurélien Gâteau wrote: Oh, version is in the request title. /me tests with 2.8. Aurélien Gâteau wrote: Just tried with 2.8.12, which is the lowest supported version IIRC (at least KF5ConfigTargets.cmake thinks so) and it works for me. Alex Merry wrote: Assuming this is linked to the previous RR on this topic, I think this is only with Visual Studio. Nope, this is on linux: cmake version 2.8.12.2 - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116696/#review52551 --- On March 10, 2014, 2 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116696/ --- (Updated March 10, 2014, 2 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- execute_process won't translate the generation expression into a path, so it refuses to work. In this case using get_property(... LOCATION) it's fine because it's only used as an imported target. Diffs - KF5ServiceMacros.cmake fd835bd Diff: https://git.reviewboard.kde.org/r/116696/diff/ Testing --- Now I can build frameworks that use kservice_desktop_to_json. 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 116650: Remove unused targets from KDocToolsMacros.cmake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116650/ --- (Updated March 10, 2014, 4:16 p.m.) Review request for Build System and KDE Frameworks. Changes --- My patch was indeed very wrong. Attempt 2: Use the full path to create the targets, rather than just the directory name. Repository: kdoctools Description --- While porting the documentation in kde-runtime I realized there was an error because when running cmake it would try to create different targets and some of them would have the same name (e.g. there is kcm/bookmarks and kio/bookmarks, and it uses the directory name to figure out the filename). I would have fixed that, but then I realised it was not running any command and nothing depended on it. Am I missing something? Diffs (updated) - KF5DocToolsMacros.cmake 6567b67 Diff: https://git.reviewboard.kde.org/r/116650/diff/ Testing --- Now kde-runtime documentation 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 116650: Give unique names to the targets created by KDocToolsMacros.cmake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116650/ --- (Updated March 10, 2014, 4:17 p.m.) Review request for Build System and KDE Frameworks. Summary (updated) - Give unique names to the targets created by KDocToolsMacros.cmake Repository: kdoctools Description --- While porting the documentation in kde-runtime I realized there was an error because when running cmake it would try to create different targets and some of them would have the same name (e.g. there is kcm/bookmarks and kio/bookmarks, and it uses the directory name to figure out the filename). I would have fixed that, but then I realised it was not running any command and nothing depended on it. Am I missing something? Diffs - KF5DocToolsMacros.cmake 6567b67 Diff: https://git.reviewboard.kde.org/r/116650/diff/ Testing --- Now kde-runtime documentation 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 116696: Fix kservice_desktop_to_json when using cmake 2.8
On March 10, 2014, 2:59 p.m., Aurélien Gâteau wrote: Strange, it works fine here. Which version of CMake are you using and which repository is failing? Aurélien Gâteau wrote: Oh, version is in the request title. /me tests with 2.8. Aurélien Gâteau wrote: Just tried with 2.8.12, which is the lowest supported version IIRC (at least KF5ConfigTargets.cmake thinks so) and it works for me. Alex Merry wrote: Assuming this is linked to the previous RR on this topic, I think this is only with Visual Studio. Aleix Pol Gonzalez wrote: Nope, this is on linux: cmake version 2.8.12.2 Aurélien Gâteau wrote: You still haven't answered the second part: which repository? Sebastian Kügler wrote: In my case, cmake-git (built with kdesrc-build). Aurélien Gâteau wrote: I meant: which framework fails to build? I think KDBusTools and KTextEditor were failing for me. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116696/#review52551 --- On March 10, 2014, 2 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116696/ --- (Updated March 10, 2014, 2 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- execute_process won't translate the generation expression into a path, so it refuses to work. In this case using get_property(... LOCATION) it's fine because it's only used as an imported target. Diffs - KF5ServiceMacros.cmake fd835bd Diff: https://git.reviewboard.kde.org/r/116696/diff/ Testing --- Now I can build frameworks that use kservice_desktop_to_json. 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 116696: Fix kservice_desktop_to_json when using cmake 2.8
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116696/ --- (Updated March 11, 2014, 10 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kservice Description --- execute_process won't translate the generation expression into a path, so it refuses to work. In this case using get_property(... LOCATION) it's fine because it's only used as an imported target. Diffs - KF5ServiceMacros.cmake fd835bd Diff: https://git.reviewboard.kde.org/r/116696/diff/ Testing --- Now I can build frameworks that use kservice_desktop_to_json. 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 116650: Give unique names to the targets created by KDocToolsMacros.cmake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116650/ --- (Updated March 11, 2014, 11:05 a.m.) Review request for Build System and KDE Frameworks. Changes --- Don't center the replace on /. Actually only allow alphanumeric characters on the target name and replace the rest with -. This should be more flexible. Repository: kdoctools Description --- While porting the documentation in kde-runtime I realized there was an error because when running cmake it would try to create different targets and some of them would have the same name (e.g. there is kcm/bookmarks and kio/bookmarks, and it uses the directory name to figure out the filename). I would have fixed that, but then I realised it was not running any command and nothing depended on it. Am I missing something? Diffs (updated) - KF5DocToolsMacros.cmake 6567b67 Diff: https://git.reviewboard.kde.org/r/116650/diff/ Testing --- Now kde-runtime documentation 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 116723: Update cmake code in apidox to reflect new macro names
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116723/#review52653 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 11, 2014, 4:10 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116723/ --- (Updated March 11, 2014, 4:10 p.m.) Review request for KDE Frameworks. Repository: kauth Description --- These are based on the commit Reformat apidocs to be manageable, which just adjusts the line breaks of the apidocs without changing the content. Update cmake code in apidox to reflect new macro names Fix doxygen warnings about not being able to resolve link requests Diffs - README.md 0cc38fd90cd1a83bae754cf9724fc0c998d42075 src/kauthactionreply.h d1fefc0243ba62d1447eecfa3c9f0cd7846a04fa Diff: https://git.reviewboard.kde.org/r/116723/diff/ Testing --- Regenerated apidocs. Only warnings are for undocumented members. Docs seem correct by visual inspection. 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 116732: Use the new uic macro
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116732/#review52663 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 11, 2014, 5:20 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116732/ --- (Updated March 11, 2014, 5:20 p.m.) Review request for KDE Frameworks and Valentin Rusu. Repository: kwallet Description --- Use the new uic macro This got missed because the line is never called currently (due to a lack of a ported QGpgMe). Diffs - tests/kwalletd/CMakeLists.txt 65e75234b1d52e519d34a47bb14521d3b1ed6482 Diff: https://git.reviewboard.kde.org/r/116732/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 116740: Fix kdeglobals location in apidox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116740/#review52701 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 11, 2014, 7:46 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116740/ --- (Updated March 11, 2014, 7:46 p.m.) Review request for KDE Frameworks. Repository: kxmlgui Description --- Fix kdeglobals location in apidox Diffs - src/kcheckaccelerators.h 7ce96430b584543bf853a0595db93c1a0a569e59 Diff: https://git.reviewboard.kde.org/r/116740/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 116650: Give unique names to the targets created by KDocToolsMacros.cmake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116650/ --- (Updated March 11, 2014, 11:14 p.m.) Status -- This change has been marked as submitted. Review request for Build System and KDE Frameworks. Repository: kdoctools Description --- While porting the documentation in kde-runtime I realized there was an error because when running cmake it would try to create different targets and some of them would have the same name (e.g. there is kcm/bookmarks and kio/bookmarks, and it uses the directory name to figure out the filename). I would have fixed that, but then I realised it was not running any command and nothing depended on it. Am I missing something? Diffs - KF5DocToolsMacros.cmake 6567b67 Diff: https://git.reviewboard.kde.org/r/116650/diff/ Testing --- Now kde-runtime documentation 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 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- src/kcompletionbox.h https://git.reviewboard.kde.org/r/116747/#comment37176 I wouldn't leave the implementation here. Move it to the .cpp file, this way it can be changed in the future, if it's required for some reason. Also there's a typo in the method name. Have you looked through the uses of the un-slotted methods? (lxr.kde.org). Maybe there's a reason for that... :/ - Aleix Pol Gonzalez On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/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 116747: Clean up KCompletionBox
On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: src/kcompletionbox.h, line 228 https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228 I wouldn't leave the implementation here. Move it to the .cpp file, this way it can be changed in the future, if it's required for some reason. Also there's a typo in the method name. David Gil Oliva wrote: Alex Merry inlined deprecated methods in https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way to go... Alex Merry wrote: Well, there's a balance to be struck: putting them in the header ensures there is no runtime cost to programs that don't use the deprecated methods, as the code should be optimised away, that the library is always binary-compatible even if you compile it with deprecated code disabled (*_NO_DEPRECATED) and makes the header code document how to replace existing calls. The downsides are an inability to fix the code later and an inability to access members of a private d-pointer class. Neither of those are an issue here, as we're just renaming the method. tl;dr: I disagree with Aleix, and think it should stay in the header. Oh, and Aleix: could you please select the whole method when you're doing a comment like this, rather than just the first line? Otherwise it's a pain to see what you're referring to. Thanks :-) Well, I wouldn't bother about runtime penalty given that it's deprecated and we shouldn't be using it anyway. Also we can't make assumptions on how things are going to be optimized. But it's ok, I don't think it's worth discussing further, I doubt this is going to be a problem in the future. On March 11, 2014, 11:17 p.m., David Gil Oliva wrote: Have you looked through the uses of the un-slotted methods? (lxr.kde.org). Maybe there's a reason for that... :/ David Gil Oliva wrote: Maybe I'm totally wrong, but I can't imagine any way that a getter can be useful as a slot. connect (widget, SLOT(valueChanged(), completionBox, SIGNAL(isTabHandling()); Does it make sense?¿?:-/ Alex Merry wrote: Non-void slots are only useful if they work as a slot (ie: have some sort of side-effect) and might be called directly or with QMetaObject::invokeMethod(). If the method is const (like a getter), there's no point having it a slot at all; if you want to be able to use it with invokeMethod, you can just make it Q_INVOKABLE. Well, having const methods as slots doesn't make sense indeed, if it's not for exposing on the meta object system, that's why I said you could do a fast lxr. I just did, didn't find anything relevant. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/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 116762: Remove new in kde4 comment
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/#review52753 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 12, 2014, 1:46 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/ --- (Updated March 12, 2014, 1:46 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Remove new in kde4 comment Diffs - src/services/kservicetypeprofile.cpp 908e76599c553a48ea509695f1d8b83be98fee1a Diff: https://git.reviewboard.kde.org/r/116762/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 116767: Clean up CMake code from pre-splitting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/#review52772 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 12, 2014, 2:57 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/ --- (Updated March 12, 2014, 2:57 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Clean up CMake code from pre-splitting Remove code that was for building as part of kdelibs, add feature_summary and respect the option to disable tests. Diffs - CMakeLists.txt 4d08bb97a569488849504e04078a71995a1fc53b Diff: https://git.reviewboard.kde.org/r/116767/diff/ Testing --- Configures, builds and installs. 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 116886: Refactor private variables of KCompletion
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116886/#review53379 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 18, 2014, 11:01 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116886/ --- (Updated March 18, 2014, 11:01 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Refactor private variables of KCompletion Also: reorder variables declaration to avoid padding Diffs - src/kcompletion_p.h e3fad26 src/kcompletion.cpp 7396029 Diff: https://git.reviewboard.kde.org/r/116886/diff/ Testing --- It builds. Autotests 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 116920: Move the autostart and wrapper docs into a docs/ subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116920/#review53513 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 20, 2014, 11:17 a.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116920/ --- (Updated March 20, 2014, 11:17 a.m.) Review request for KDE Frameworks. Repository: kinit Description --- Move the autostart and wrapper docs into a docs/ subdirectory Diffs - README.autostart README.wrapper Diff: https://git.reviewboard.kde.org/r/116920/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 116939: Add deprecation info to kcombobox, kcompletionbase and klineedit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116939/#review53625 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 20, 2014, 11:16 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116939/ --- (Updated March 20, 2014, 11:16 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- See summary Diffs - src/kcombobox.h eea930d src/kcompletionbase.h 8022214 src/klineedit.h 76a1f01 Diff: https://git.reviewboard.kde.org/r/116939/diff/ Testing --- 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 116866: Use std::isnan on compilers that support it (fixes MinGW on Windows)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116866/#review53877 --- src/colors/kcolorutils.cpp https://git.reviewboard.kde.org/r/116866/#comment37768 Wouldn't it make more sense to use qIsNan? http://qt-project.org/doc/qt-5.0/qtcore/qtglobal.html#qIsNaN - Aleix Pol Gonzalez On March 22, 2014, 8:54 p.m., Michael Hansen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116866/ --- (Updated March 22, 2014, 8:54 p.m.) Review request for KDE Frameworks. Repository: kguiaddons Description --- Use std::isnan from cmath instead of isnan from math.h, as MinGW-32 on Windows does not include the latter. This keeps the _isnan hack for MSVC, since that compiler doesn't include either standard version :(. Diffs - src/kguiaddons_config.h.cmake PRE-CREATION src/colors/kcolorutils.cpp 7df25b3d7acbb65b29513d2139d7b83de53ee4c2 src/ConfigureChecks.cmake PRE-CREATION src/CMakeLists.txt 624d2e109be5c26af9781101a005b4a163361a92 Diff: https://git.reviewboard.kde.org/r/116866/diff/ Testing --- Compiled with MSVC10 (32-bit), MinGW 4.8 (32-bit, Windows native), and GCC 4.8 (Arch x86_64). Thanks, Michael Hansen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117011: Use bin/../share on Windows as a workaround
On March 23, 2014, 11:51 p.m., Aleix Pol Gonzalez wrote: So what should happen so that we didn't need the workaround? Alexander Richardson wrote: QStandardPaths would have to look in %KDEROOT%\share and not just C:\ProgramData. So why aren't we adding this in Qt? Or even, why are we installing documentation in share/ if it doesn't make sense on Windows? - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117011/#review53878 --- On March 23, 2014, 11:28 p.m., Andrius da Costa Ribas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117011/ --- (Updated March 23, 2014, 11:28 p.m.) Review request for KDE Frameworks and kdewin. Repository: kdoctools Description --- This is a workaround for kdoctools for the same problem discussed in review #115210. In particular, emerge installs docbook-dtd data under %KDEROOT%\share. Diffs - src/xslt.cpp db67599 Diff: https://git.reviewboard.kde.org/r/117011/diff/ Testing --- Tested using MSVC 2013. KJsEmbed, which depends on this, builds after this patch (KJsEmbed still has an unrelated problem on install step). Thanks, Andrius da Costa Ribas ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117017: Remove forward headers for KTextEditor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117017/#review53937 --- There's a slight difference, and the reason we haven't been doing this for most headers. From KDE4Support you can include headers prefixing KDE/ (such as KDE/KTextEditor/MovingRange). If you remove these that won't be possible anymore thus making porting slightly harder. Arguably people won't be doing KDE/KTextEditor, but then I don't know why people added the KDE/ at all... - Aleix Pol Gonzalez On March 24, 2014, 10:48 a.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117017/ --- (Updated March 24, 2014, 10:48 a.m.) Review request for KDE Frameworks and Dominik Haumann. Repository: kde4support Description --- Remove forward headers for KTextEditor Some of the headers have actually been removed already. Keeping broken forward headers actually makes it *more* difficult to port. Diffs - src/includes/KTextEditor/CommandExtension 4187c61882a83cab906fa87cd16bd18229b6efb5 src/includes/KTextEditor/Command 4187c61882a83cab906fa87cd16bd18229b6efb5 src/includes/KTextEditor/CodeCompletionModelControllerInterface a92ceb40a0d0afbf42d8b3302492b8e52a7f8505 src/includes/KTextEditor/CodeCompletionModel 3511d00b212e5c56613bf4f06fedc5a5d76cb3bc src/includes/CMakeLists.txt bc7d00f85012ec436937fd3d402e9c08e28f6b74 src/includes/KTextEditor/Attribute 6420e896e93532188d08894853176842c7d8ccae src/includes/KTextEditor/CodeCompletionInterface 41341c38dd92e7c1533b0ba74eceb735408a1d3f src/includes/KTextEditor/Document 858d360f8ae751c16b03d350d7e415bea400906d src/includes/KTextEditor/Cursor 2811cda0f69b3c263ac8b2dd210b50f6239f7ff2 src/includes/KTextEditor/ConfigPage b3904bee10ffc0245bca1a928389237813850ec3 src/includes/KTextEditor/ConfigInterface 0617835fc7621c4c26a2d50ca95d12d8870fffc2 src/includes/KTextEditor/CommandInterface 4187c61882a83cab906fa87cd16bd18229b6efb5 src/includes/KTextEditor/ModificationInterface 50df2902648156dc5cd4630f587add36d320a43a src/includes/KTextEditor/MessageInterface 41d9aa45a8edb7b9b50e0b82c7b113b6e07bcb32 src/includes/KTextEditor/Editor 76d55675c78248875996b0284288a34af303e8c7 src/includes/KTextEditor/HighlightInterface 8c8c94ec679877be7e3965eba86498f06b67a883 src/includes/KTextEditor/MarkInterface 87adf561b38045bdd65fc3f64f24311aa901d8ee src/includes/KTextEditor/Message 41d9aa45a8edb7b9b50e0b82c7b113b6e07bcb32 src/includes/KTextEditor/ParameterizedSessionConfigInterface 8c26b7d418e81072df4e7dffe0038d7fdc1e3010 src/includes/KTextEditor/MovingRange 89f68605ffa863862476831cfb836660a70e4931 src/includes/KTextEditor/MovingInterface bde16eaca7d68517ce7c2068186eb641daf6eab1 src/includes/KTextEditor/MovingCursor 7c9eb3074e4feace09cca78962caf1ff27bd6394 src/includes/KTextEditor/View 411c995972b47e7a2ad4a385a6924e3a67f8c892 src/includes/KTextEditor/VariableInterface 126a93691700fab3134400907d0ba93a4e275f0d src/includes/KTextEditor/TextHintInterface 6b05d9a4a45ac8807294599e04c7dc15db076cf0 src/includes/KTextEditor/TemplateInterface2 de9d9451796710756287bfc2a627f7ae43a006b1 src/includes/KTextEditor/TemplateInterface 0142d17815fabb9136836effa61114aaa1994635 src/includes/KTextEditor/SessionConfigInterface 8c26b7d418e81072df4e7dffe0038d7fdc1e3010 src/includes/KTextEditor/Range 15aad643ee6f1f95b96f579bc66cc84f4873d006 src/includes/KTextEditor/SearchInterface f7dffc91739e82cceffea35de0632cb19e92ab0a src/includes/KTextEditor/Plugin 1016b2e5c5f930afcceb1110b00468ee1158cf7e Diff: https://git.reviewboard.kde.org/r/117017/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117012: Place KJsEmbed camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117012/#review53938
---
Then you should modify the target_include_directories INSTALL_INTERFACE data,
no?
- Aleix Pol Gonzalez
On March 24, 2014, 1:33 a.m., Andrius da Costa Ribas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117012/
---
(Updated March 24, 2014, 1:33 a.m.)
Review request for KDE Frameworks and kdewin.
Repository: kjsembed
Description
---
Currently kjsembed CMake file tries to install both
${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and
${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not
allowed in a case-insensitive filesystem, causing the install step to fail on
Windows.
Diffs
-
src/kjsembed/CMakeLists.txt e0ab74c
Diff: https://git.reviewboard.kde.org/r/117012/diff/
Testing
---
Tested using MSVC 2013
Thanks,
Andrius da Costa Ribas
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117016: Allow overriding DrKonqi lookup directories by PATH
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/#review53939 --- What about QStandardPaths::findExecutable? Actually this one should look into libexec too (at least the equivalent KStandardDirs::findExe used to). - Aleix Pol Gonzalez On March 24, 2014, 10:52 a.m., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/ --- (Updated March 24, 2014, 10:52 a.m.) Review request for KDE Frameworks. Repository: kcrash Description --- Since KCrash is a framework that relies on DrKonqi binary being provided by a 3rd party software (kde-runtime), it should not make assumptions regarding location of the utility. This patch makes KCrash to look for drkonqi binary first in $PATH, then falling back to CMAKE_INSTALL_PREFIX/LIBEXEC_INSTALL_DIR. With this patch it's possible for distributions to ship KDE Frameworks in normal prefix (/usr), but have current snapshots of kde-runtime in /opt/kde5 for instance. Diffs - src/kcrash.cpp 87163cc Diff: https://git.reviewboard.kde.org/r/117016/diff/ Testing --- - Installed KCrash into /usr prefix - Installed drkonqi from kde-runtime master to /opt/kde5 prefix - started broken application - no could not find drkonqi warning anymore - crashed application, got drkonqi window Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117018: Use qIsNaN() instead of isnan()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117018/#review53968 --- Ship it! Less code, less ifdef's, more happiness! - Aleix Pol Gonzalez On March 24, 2014, 12:10 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117018/ --- (Updated March 24, 2014, 12:10 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez and Michael Hansen. Repository: kguiaddons Description --- Use qIsNaN() instead of isnan() Revert Use std::isnan on compilers that support it This reverts commit b0b7e84f75f9b9edfbe9ef8b60f56f85c4a7800c. As apol pointed out, qIsNaN would be a better choice. Fun fact: the Qt docs claim qIsNaN() is defined in QtGlobal, but you actually have to include QtNumeric. Diffs - src/CMakeLists.txt f15264e01849b00907afedc262078225ed8756a5 src/ConfigureChecks.cmake c194dfc5ce54c8c7fb3b0475a704f3165ce5cca1 src/colors/kcolorutils.cpp 5f4d0fdeb77d40d42b824c99e08e208aed6f2b93 src/kguiaddons_config.h.cmake 8d5cb897a7e431be298c2d290369b14dbaaa4904 Diff: https://git.reviewboard.kde.org/r/117018/diff/ Testing --- Builds, installs, tests pass (Linux, GCC 4.8). 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 117016: Allow overriding DrKonqi lookup directories by PATH
On March 24, 2014, 3:41 p.m., Alex Merry wrote: The correct solution is to get drkonqi merged into kcrash (see http://community.kde.org/Frameworks/Epics/New_Runtime_Organization). Agreed. If somebody has the time, it would be interesting to figure out what can be uncommented (see commit f8b59b6c in kde-runtime). There's many things commented out and I'm unsure what to do with it. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/#review53985 --- On March 24, 2014, 12:01 p.m., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/ --- (Updated March 24, 2014, 12:01 p.m.) Review request for KDE Frameworks. Repository: kcrash Description --- Since KCrash is a framework that relies on DrKonqi binary being provided by a 3rd party software (kde-runtime), it should not make assumptions regarding location of the utility. This patch makes KCrash to look for drkonqi binary first in $PATH, then falling back to CMAKE_INSTALL_PREFIX/LIBEXEC_INSTALL_DIR. With this patch it's possible for distributions to ship KDE Frameworks in normal prefix (/usr), but have current snapshots of kde-runtime in /opt/kde5 for instance. Diffs - src/kcrash.cpp 87163cc Diff: https://git.reviewboard.kde.org/r/117016/diff/ Testing --- - Installed KCrash into /usr prefix - Installed drkonqi from kde-runtime master to /opt/kde5 prefix - started broken application - no could not find drkonqi warning anymore - crashed application, got drkonqi window Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117037: Refactor parameter names
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117037/#review54052 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 24, 2014, 11:17 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117037/ --- (Updated March 24, 2014, 11:17 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Refactor parameter names Also add #ifndef KCOMPLETION_NO_DEPRECATED around the implementation of KLineEdit::setUrlDropsEnabled() Diffs - src/kcombobox.h 80d6573 src/kcombobox.cpp cbb209e src/kcompletion.h 3b3b0a6 src/kcompletion.cpp 847cff3 src/kcompletionbox.h f396746 src/kcompletionbox.cpp 7753643 src/khistorycombobox.h d1f5eac src/khistorycombobox.cpp 80261ae src/klineedit.h 723cb42 src/klineedit.cpp b426767 src/klineedit_p.h 397abc2 Diff: https://git.reviewboard.kde.org/r/117037/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 117042: Make KLineEdit::completionBox virtual so that konq can reimplement it
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117042/#review54053 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 24, 2014, 11:28 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117042/ --- (Updated March 24, 2014, 11:28 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Make KLineEdit::completionBox virtual so that konq can reimplement it (as suggested in a comment) Diffs - src/klineedit.h 723cb42 Diff: https://git.reviewboard.kde.org/r/117042/diff/ Testing --- 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 117059: Explicitly state link interface for libKF5Activites
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117059/#review54131 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 25, 2014, 5:35 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117059/ --- (Updated March 25, 2014, 5:35 p.m.) Review request for KDE Frameworks and Plasma. Repository: kactivities Description --- Explicitly state link interface for libKF5Activites Diffs - src/lib/core/CMakeLists.txt 7edd2e34bc5ccec6ae526c0305d090afdaf9f306 Diff: https://git.reviewboard.kde.org/r/117059/diff/ Testing --- Compiles, installs. plasa-framework, kde-runtime and kde-workspace then also build. 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 117060: Explicitly specify link interface libraries for libKF5PlasmaQuick
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117060/#review54132 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 25, 2014, 5:36 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117060/ --- (Updated March 25, 2014, 5:36 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Explicitly specify link interface libraries for libKF5PlasmaQuick The headers may not be public (yet?), but it doesn't hurt to have this stuff specified properly. Diffs - src/plasmaquick/CMakeLists.txt 83af22ecaf235c26115aa741d3c288bc71c269e9 Diff: https://git.reviewboard.kde.org/r/117060/diff/ Testing --- Builds and installs. kde-runtime and kde-workspace then also build. 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 117071: Re-enable i18n in KPluginLoader
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117071/#review54148 --- Ship it! Who would have thought that TEMP_KF5_REENABLE would be a thing... I just checked and it's being used in kde4support as well. - Aleix Pol Gonzalez On March 26, 2014, 12:15 a.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117071/ --- (Updated March 26, 2014, 12:15 a.m.) Review request for KDE Frameworks. Repository: kservice Description --- Re-enable i18n in KPluginLoader Diffs - src/plugin/kpluginloader.cpp a559529b2d370907b4932e341294fc6c16dd317e Diff: https://git.reviewboard.kde.org/r/117071/diff/ Testing --- Builds, installs. 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 117072: Add autotests for KPluginLoader and KPluginFactory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117072/#review54150 --- Ship it! Yay unit tests!! - Aleix Pol Gonzalez On March 26, 2014, 2:22 a.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117072/ --- (Updated March 26, 2014, 2:22 a.m.) Review request for KDE Frameworks. Repository: kservice Description --- Add autotests for KPluginLoader and KPluginFactory Diffs - autotests/CMakeLists.txt 220aa14bb1a38e0d5d822c93b9a110040488601d autotests/kpluginfactorytest.cpp PRE-CREATION autotests/kpluginloadertest.cpp PRE-CREATION autotests/multiplugin.h PRE-CREATION autotests/multiplugin.cpp PRE-CREATION autotests/nsaplugin.cpp 3c8fb47ad001c6760726564ba1815e3eeecb1502 autotests/unversionedplugin.h PRE-CREATION autotests/unversionedplugin.cpp PRE-CREATION autotests/versionedplugin.h PRE-CREATION autotests/versionedplugin.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117072/diff/ Testing --- Builds, tests pass. 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 117089: Use Q_DECL_EXPORT instread of KSERVICE_EXPORT in K_EXPORT_PLUGIN_VERSION
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117089/#review54215 --- Ship it! I think it makes sense, it's not linking with KService. One of the good things of testing different compilers. :) - Aleix Pol Gonzalez On March 26, 2014, 4:05 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117089/ --- (Updated March 26, 2014, 4:05 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Use Q_DECL_EXPORT instread of KSERVICE_EXPORT in K_EXPORT_PLUGIN_VERSION Using KSERVICE_EXPORT causes errors on Windows since it expands to __declspec(dllimport) or __declspec(dllexport) depending on whether it is being build or the header is being used. Since we are not building KService when K_EXPORT_PLUGIN_VERSION is being used we get the following error: error C2491: 'kde_plugin_version' : definition of dllimport data not allowed Diffs - src/plugin/kexportplugin.h 15d9d7fd91b0b6ed7f32a422a18c61f16bee34bd Diff: https://git.reviewboard.kde.org/r/117089/diff/ Testing --- compiles now 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 116969: Deprecate reset() in KHistoryComboBox and substitute it for resetIndex()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116969/#review54254 --- I like the cleaning up, but maybe you're being too picky for preferring resetIndex to reset? - Aleix Pol Gonzalez On March 24, 2014, 9:45 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116969/ --- (Updated March 24, 2014, 9:45 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Deprecate reset() and substitute it for resetIndex(). Remove private reset() and move the implementation to the public reset() Move method to public Q_SLOTS section Diffs - autotests/kcombobox_unittest.cpp 1479ae2 src/khistorycombobox.h d1f5eac src/khistorycombobox.cpp 80261ae Diff: https://git.reviewboard.kde.org/r/116969/diff/ Testing --- It builds. 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 117104: Remove KLineEdit::clear(): it doesn't seem to modify clipboard anymore
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117104/#review54255 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 26, 2014, 11:15 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117104/ --- (Updated March 26, 2014, 11:15 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Remove KLineEdit::clear(): it doesn't seem to modify clipboard anymore From the comment, I understand that formerly the clipboard changed to the contents of the KLineEdit after using clear(). I wrote a little stupid program and checked that the clipboard doesn't change using QLineEdit::clear(). Did I understand correctly? Diffs - src/klineedit.h ca7e105 src/klineedit.cpp f417902 Diff: https://git.reviewboard.kde.org/r/117104/diff/ Testing --- It builds. The clipboard doesn't change. File Attachments kline.h https://git.reviewboard.kde.org/media/uploaded/files/2014/03/26/fdc5ab2a-01ab-4fec-b88a-8c694d562da9__kline.h kline.cpp https://git.reviewboard.kde.org/media/uploaded/files/2014/03/26/9e4e8cc1-c7d5-4b72-9058-5475a8ac94b9__kline.cpp 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 117108: Deprecate KTextEdit clickmessage and use QTextEdit placeholder
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117108/#review54284 --- Ship it! Less code! - Aleix Pol Gonzalez On March 27, 2014, 10:08 a.m., Benjamin Port wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117108/ --- (Updated March 27, 2014, 10:08 a.m.) Review request for KDE Frameworks. Repository: ktextwidgets Description --- Deprecate clickmessage and use QTextEdit placeholder. We lose ability to put text in italic. Diffs - src/widgets/ktextedit.h 36786bb src/widgets/ktextedit.cpp 9bc8b58 Diff: https://git.reviewboard.kde.org/r/117108/diff/ Testing --- Thanks, Benjamin Port ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117016: Allow overriding DrKonqi lookup directories by PATH
On March 24, 2014, 3:41 p.m., Alex Merry wrote: The correct solution is to get drkonqi merged into kcrash (see http://community.kde.org/Frameworks/Epics/New_Runtime_Organization). Aleix Pol Gonzalez wrote: Agreed. If somebody has the time, it would be interesting to figure out what can be uncommented (see commit f8b59b6c in kde-runtime). There's many things commented out and I'm unsure what to do with it. Dan Vrátil wrote: I agree, however until it is done, this patch would make lifes of distribution packagers a bit easier :-) Kevin Ottens wrote: Aleix, weren't you indeed planning to move drkonqi in? Any ETA? I wanted to figure it out first. At the moment Dr Konqi depends on some commented out kdepimlibs bits I'm unsure what to do with, so I decided to lower it's priority. I can do it before next week if it's considered important. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/#review53985 --- On March 24, 2014, 12:01 p.m., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/ --- (Updated March 24, 2014, 12:01 p.m.) Review request for KDE Frameworks. Repository: kcrash Description --- Since KCrash is a framework that relies on DrKonqi binary being provided by a 3rd party software (kde-runtime), it should not make assumptions regarding location of the utility. This patch makes KCrash to look for drkonqi binary first in $PATH, then falling back to CMAKE_INSTALL_PREFIX/LIBEXEC_INSTALL_DIR. With this patch it's possible for distributions to ship KDE Frameworks in normal prefix (/usr), but have current snapshots of kde-runtime in /opt/kde5 for instance. Diffs - src/kcrash.cpp 87163cc Diff: https://git.reviewboard.kde.org/r/117016/diff/ Testing --- - Installed KCrash into /usr prefix - Installed drkonqi from kde-runtime master to /opt/kde5 prefix - started broken application - no could not find drkonqi warning anymore - crashed application, got drkonqi window Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 117122: Cut the dependency between country files and KIO
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117122/ --- Review request for KDE Frameworks, Albert Astals Cid and John Layt. Repository: kio Description --- I was going through the kde-runtime localization thread again [1] and decided to start looking into it. As Albert pointed out, there's many modules making use of these, one would be KIO. I looked into the actual dependency and saw that it's rather small so decided to just drop it. In this case it's using the country/entry.desktop file to check what binary format to use, but then it seems like no entry.desktop file is specifying it [2], so it's a bit of an absurd configuration settings. I just changed it to stop reading the value. [1] https://www.mail-archive.com/kde-frameworks-devel@kde.org/msg11839.html [2] ~/kde5/share/locale/l10n$ grep -R BinaryUnitDialect C/entry.desktop:BinaryUnitDialect=0 Diffs - src/core/global.cpp 99ab2e7 Diff: https://git.reviewboard.kde.org/r/117122/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 117052: Add ECMSetupQtTranslations
On March 27, 2014, 8:31 p.m., Alex Merry wrote: modules/ECMTrLoader.cpp.in, line 19 https://git.reviewboard.kde.org/r/117052/diff/2/?file=257837#file257837line19 QLatin1String() when you're using + Why? That doesn't make sense on my book. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117052/#review54369 --- On March 27, 2014, 3:26 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117052/ --- (Updated March 27, 2014, 3:26 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- This simplifies translation handling for frameworks using Qt translation system. Diffs - modules/ECMSetupQtTranslations.cmake PRE-CREATION modules/ECMTrLoader.cpp.in PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117052/diff/ Testing --- Here is a patch which make kbookmarks use it: http://agateau.com/tmp/kbookmarks-tr.diff . The necessary changes for the Messages.sh part of the patch are being reviewed for inclusion in the l10n-kde4 repository. 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 117087: co-installability: rename entry.desktop to kf5_entry.desktop
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117087/#review54408 --- Again, these will go to kde4support, calling them kf5_ is weird... Additionally, these are used in many places. These should be updated as well. - Aleix Pol Gonzalez On March 26, 2014, 1:44 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117087/ --- (Updated March 26, 2014, 1:44 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: kde4support Description --- stop clashing files on install with kdelibs4 and rename entry.desktop to kf5_entry.desktop Diffs - src/CMakeLists.txt 307cafa src/kdecore/klocale_kde.cpp d1bedee Diff: https://git.reviewboard.kde.org/r/117087/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 117132: Use QLocale for figuring out what languages we're translated into
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117132/ --- Review request for KDE Frameworks, Albert Astals Cid and Chusslove Illich. Repository: kxmlgui Description --- Instead of asking the file-system what languages the application is translated into, ask QLocale what languages we have available instead. Diffs - src/khelpmenu.cpp 4f6ce7b Diff: https://git.reviewboard.kde.org/r/117132/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
Review Request 117134: Use QLocale to list all languages
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117134/ --- Review request for KDE Frameworks, Albert Astals Cid and Chusslove Illich. Repository: kconfigwidgets Description --- Instead of finding the entries.desktop files in the file-system, use QLocale languages to list all languages. Anybody knows if bcp47 compatible with our language naming scheme yet? Diffs - src/klanguagebutton.cpp 9f0c055 Diff: https://git.reviewboard.kde.org/r/117134/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 117136: Expose KAssistantDialog buttons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117136/ --- (Updated March 28, 2014, 2:56 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- Fix API regression in KAssistantDialog in KWidgetsAddons. The previous, next and finish button used to be exposed through the KDialog::UserN buttons. This is not a KDialog anymore and QDialogButtonBox doesn't expose these. This patch exposes the buttons in 3 different methods. Diffs - src/kassistantdialog.cpp 00895f0 src/kassistantdialog.h e7ffaf5 Diff: https://git.reviewboard.kde.org/r/117136/diff/ Testing --- Using it from the ongoing port of dr konqi. 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 117016: Allow overriding DrKonqi lookup directories by PATH
On March 28, 2014, 2:13 p.m., David Faure wrote: How does findExecutable() help at all? You plan on adding libexec to your $PATH? KStandardDirs::findExe used to look into libexec, IIRC. Maybe we could make it work. Or do something completely different. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/#review54443 --- On March 24, 2014, 12:01 p.m., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/ --- (Updated March 24, 2014, 12:01 p.m.) Review request for KDE Frameworks. Repository: kcrash Description --- Since KCrash is a framework that relies on DrKonqi binary being provided by a 3rd party software (kde-runtime), it should not make assumptions regarding location of the utility. This patch makes KCrash to look for drkonqi binary first in $PATH, then falling back to CMAKE_INSTALL_PREFIX/LIBEXEC_INSTALL_DIR. With this patch it's possible for distributions to ship KDE Frameworks in normal prefix (/usr), but have current snapshots of kde-runtime in /opt/kde5 for instance. Diffs - src/kcrash.cpp 87163cc Diff: https://git.reviewboard.kde.org/r/117016/diff/ Testing --- - Installed KCrash into /usr prefix - Installed drkonqi from kde-runtime master to /opt/kde5 prefix - started broken application - no could not find drkonqi warning anymore - crashed application, got drkonqi window Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 117136: Expose KAssistantDialog buttons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117136/ --- Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- Fix API regression in KAssistantDialog in KWidgetsAddons. The previous, next and finish button used to be exposed through the KDialog::UserN buttons. This is not a KDialog anymore and QDialogButtonBox doesn't expose these. This patch exposes the buttons in 3 different methods. Diffs - src/kassistantdialog.cpp 00895f0 src/kassistantdialog.h e7ffaf5 Diff: https://git.reviewboard.kde.org/r/117136/diff/ Testing --- Using it from the ongoing port of dr konqi. 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 117016: Allow overriding DrKonqi lookup directories by PATH
On March 24, 2014, 3:41 p.m., Alex Merry wrote: The correct solution is to get drkonqi merged into kcrash (see http://community.kde.org/Frameworks/Epics/New_Runtime_Organization). Aleix Pol Gonzalez wrote: Agreed. If somebody has the time, it would be interesting to figure out what can be uncommented (see commit f8b59b6c in kde-runtime). There's many things commented out and I'm unsure what to do with it. Dan Vrátil wrote: I agree, however until it is done, this patch would make lifes of distribution packagers a bit easier :-) Kevin Ottens wrote: Aleix, weren't you indeed planning to move drkonqi in? Any ETA? Aleix Pol Gonzalez wrote: I wanted to figure it out first. At the moment Dr Konqi depends on some commented out kdepimlibs bits I'm unsure what to do with, so I decided to lower it's priority. I can do it before next week if it's considered important. Dan Vrátil wrote: Initial port of KXMLRPCClient to frameworks is already done. It's very tiny (2 classes), so it could be properly frameworkized on the PIM sprint this weekend. It depends on KIO ATM for doing HTTP POST requests, but maybe it could be replaced by a Qt solution, so that it could go to Tier 1. Aleix Pol Gonzalez wrote: I think we're a bit late for that. We can do it but the addition of frameworks is frozen AFAIK. But yes, kdepimlibs should be sorted out ASAP and this sprint can be a good moment for doing so. Alex Merry wrote: As a temporary solution, the two classes could be just dropped into KCrash, but without exporting them. I've been looking into it, I made it compile completely by using KF5XmlRpcClient (the port of kdepimlibs is in a much better state than I expected, thanks!!). I'm unsure we want to provide the bugzilla integration together with drkonqi, so I've been thinking that we probably want to split it out into a plugin. In any case, this is quite unrelated to the subject, given that either in kcrash or in kde-runtime, this will have to be installed in libexec. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/#review53985 --- On March 24, 2014, 12:01 p.m., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117016/ --- (Updated March 24, 2014, 12:01 p.m.) Review request for KDE Frameworks. Repository: kcrash Description --- Since KCrash is a framework that relies on DrKonqi binary being provided by a 3rd party software (kde-runtime), it should not make assumptions regarding location of the utility. This patch makes KCrash to look for drkonqi binary first in $PATH, then falling back to CMAKE_INSTALL_PREFIX/LIBEXEC_INSTALL_DIR. With this patch it's possible for distributions to ship KDE Frameworks in normal prefix (/usr), but have current snapshots of kde-runtime in /opt/kde5 for instance. Diffs - src/kcrash.cpp 87163cc Diff: https://git.reviewboard.kde.org/r/117016/diff/ Testing --- - Installed KCrash into /usr prefix - Installed drkonqi from kde-runtime master to /opt/kde5 prefix - started broken application - no could not find drkonqi warning anymore - crashed application, got drkonqi window Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117087: co-installability: rename entry.desktop to kf5_entry.desktop
On March 28, 2014, 4:16 p.m., Commit Hook wrote: This review has been submitted with commit a33377b84b9b312cf5f1f2baae42e6e514731d64 by Jonathan Riddell to branch master. Why did you ignore my comment? This breaks entry.desktop files usage. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117087/#review54467 --- On March 28, 2014, 4:16 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117087/ --- (Updated March 28, 2014, 4:16 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: kde4support Description --- stop clashing files on install with kdelibs4 and rename entry.desktop to kf5_entry.desktop Diffs - src/CMakeLists.txt 307cafa src/kdecore/klocale_kde.cpp d1bedee Diff: https://git.reviewboard.kde.org/r/117087/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 117134: Use QLocale to list all languages
On March 28, 2014, 3:51 p.m., David Faure wrote: Same problem as https://git.reviewboard.kde.org/r/117132/ : this code is supposed to list the available translations. The new code doesn't do that. Is was using entry.desktop, maybe it can look up myapp.gmo files instead. But it has to be based on what is actually installed. Not really, it's looking up all languages (as they are installed by kde-runtime). All users should have all entry.desktop files, not only the ones that you have available for translation, AFAIU. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117134/#review54465 --- On March 28, 2014, 11:50 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117134/ --- (Updated March 28, 2014, 11:50 a.m.) Review request for KDE Frameworks, Albert Astals Cid and Chusslove Illich. Repository: kconfigwidgets Description --- Instead of finding the entries.desktop files in the file-system, use QLocale languages to list all languages. Anybody knows if bcp47 compatible with our language naming scheme yet? Diffs - src/klanguagebutton.cpp 9f0c055 Diff: https://git.reviewboard.kde.org/r/117134/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 117132: Use QLocale for figuring out what languages we're translated into
On March 28, 2014, 3:43 p.m., David Faure wrote: Looks wrong, QLocale looks at .ts/.qm files while we mostly use .po/.gmo files - different translation system. Also doubly wrong because uiLanguages() returns the user preferences (e.g. for me en, fr), which has nothing to do with how many languages are actually installed (e.g. there could be about 54). Then should we get a method to ask KLocalizedString what languages we have available for the application. In any case entry.desktop files are installed at bulk by kde-runtime, so it's actually already broken in KDE4. Actually, I get to ask to switch language and then I get to only choose the one. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117132/#review54463 --- On March 28, 2014, 11:21 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117132/ --- (Updated March 28, 2014, 11:21 a.m.) Review request for KDE Frameworks, Albert Astals Cid and Chusslove Illich. Repository: kxmlgui Description --- Instead of asking the file-system what languages the application is translated into, ask QLocale what languages we have available instead. Diffs - src/khelpmenu.cpp 4f6ce7b Diff: https://git.reviewboard.kde.org/r/117132/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 117087: co-installability: rename entry.desktop to kf5_entry.desktop
On March 28, 2014, 4:16 p.m., Commit Hook wrote: This review has been submitted with commit a33377b84b9b312cf5f1f2baae42e6e514731d64 by Jonathan Riddell to branch master. Aleix Pol Gonzalez wrote: Why did you ignore my comment? This breaks entry.desktop files usage. Jonathan Riddell wrote: Sorry I was wanting to get it in for the KF5 beta, I'll post a patch for kde-runtime shortly, anything else I should be updating? There's a list of uses of these files in this thread: https://www.mail-archive.com/kde-frameworks-devel@kde.org/msg11839.html - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117087/#review54467 --- On March 28, 2014, 4:16 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117087/ --- (Updated March 28, 2014, 4:16 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: kde4support Description --- stop clashing files on install with kdelibs4 and rename entry.desktop to kf5_entry.desktop Diffs - src/CMakeLists.txt 307cafa src/kdecore/klocale_kde.cpp d1bedee Diff: https://git.reviewboard.kde.org/r/117087/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
