Re: Review Request 123768: port to new ecm_add_test PROPERTIES argument
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123768/#review80283 --- Ship it! Ship It! - Jan Grulich On Kvě. 13, 2015, 9 dop., Harald Sitter wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123768/ --- (Updated Kvě. 13, 2015, 9 dop.) Review request for KDE Frameworks and Network Management. Repository: modemmanager-qt Description --- port to new ecm_add_test PROPERTIES argument this depends on r123722 getting landed Diffs - autotests/CMakeLists.txt 03e19393ab5ba2f4c1e5fbd0d76ff7378eb88ae3 Diff: https://git.reviewboard.kde.org/r/123768/diff/ Testing --- make ctest -j9 Thanks, Harald Sitter ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 123768: port to new ecm_add_test PROPERTIES argument
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123768/ --- Review request for KDE Frameworks and Network Management. Repository: modemmanager-qt Description --- port to new ecm_add_test PROPERTIES argument this depends on r123722 getting landed Diffs - autotests/CMakeLists.txt 03e19393ab5ba2f4c1e5fbd0d76ff7378eb88ae3 Diff: https://git.reviewboard.kde.org/r/123768/diff/ Testing --- make ctest -j9 Thanks, Harald Sitter ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123742: knewstuff: Add verbose ecm message when ECM isn't found.
On May 13, 2015, 7:10 a.m., Aleix Pol Gonzalez wrote: CMakeLists.txt, line 8 https://git.reviewboard.kde.org/r/123742/diff/3/?file=368712#file368712line8 I insist, feature_sumary goes at the bottom. (and is already there) Is it needed to have it twice? Yes, otherwise we only get the generic cmake failure. Actually, let me check... Maybe it only means we get a bunch of failures for the include calls in addition to the verbose ECM message. - Jeremy --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/#review80299 --- On May 13, 2015, 7 a.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/ --- (Updated May 13, 2015, 7 a.m.) Review request for KDE Frameworks and Jeremy Whiting. Repository: knewstuff Description --- Make ECM missing message explain a) what ECM is, and b) where to find it. Diffs - CMakeLists.txt cb23ccb8a9b0421a554b41234c3d9ced3965d378 Diff: https://git.reviewboard.kde.org/r/123742/diff/ Testing --- KNewStuff (and any other framework we add these changes to) now reports the ECM url and name when it isn't found at cmake time. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123742: knewstuff: Add verbose ecm message when ECM isn't found.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/ --- (Updated May 13, 2015, 7 a.m.) Review request for KDE Frameworks and Jeremy Whiting. Changes --- Reordered lines as per Kevin's suggestion. I agree this looks a bit better. Repository: knewstuff Description --- Make ECM missing message explain a) what ECM is, and b) where to find it. Diffs (updated) - CMakeLists.txt cb23ccb8a9b0421a554b41234c3d9ced3965d378 Diff: https://git.reviewboard.kde.org/r/123742/diff/ Testing --- KNewStuff (and any other framework we add these changes to) now reports the ECM url and name when it isn't found at cmake time. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123742: knewstuff: Add verbose ecm message when ECM isn't found.
On May 13, 2015, 7:10 a.m., Aleix Pol Gonzalez wrote: CMakeLists.txt, line 8 https://git.reviewboard.kde.org/r/123742/diff/3/?file=368712#file368712line8 I insist, feature_sumary goes at the bottom. (and is already there) Is it needed to have it twice? Jeremy Whiting wrote: Yes, otherwise we only get the generic cmake failure. Actually, let me check... Maybe it only means we get a bunch of failures for the include calls in addition to the verbose ECM message. Nope. If we don't add the extra feature_summary before we start including other stuff it looks like this with ECM missing which defeats the purpose of this change imo: CMake Warning at CMakeLists.txt:7 (find_package): Could not find a package configuration file provided by ECM (requested version 5.10.0) with any of the following names: ECMConfig.cmake ecm-config.cmake Add the installation prefix of ECM to CMAKE_PREFIX_PATH or set ECM_DIR to a directory containing one of the above files. If ECM provides a separate development package or SDK, be sure it has been installed. CMake Error at CMakeLists.txt:12 (include): include could not find load file: ECMSetupVersion CMake Error at CMakeLists.txt:13 (include): include could not find load file: ECMGenerateHeaders CMake Error at CMakeLists.txt:14 (include): include could not find load file: ECMPackageConfigHelpers CMake Error at CMakeLists.txt:15 (include): include could not find load file: KDEInstallDirs CMake Error at CMakeLists.txt:16 (include): include could not find load file: KDEFrameworkCompilerSettings CMake Error at CMakeLists.txt:17 (include): include could not find load file: KDECMakeSettings CMake Error at CMakeLists.txt:21 (ecm_setup_version): Unknown CMake command ecm_setup_version. -- Configuring incomplete, errors occurred! See also /home/jeremy/attica/build/CMakeFiles/CMakeOutput.log. - Jeremy --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/#review80299 --- On May 13, 2015, 7 a.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/ --- (Updated May 13, 2015, 7 a.m.) Review request for KDE Frameworks and Jeremy Whiting. Repository: knewstuff Description --- Make ECM missing message explain a) what ECM is, and b) where to find it. Diffs - CMakeLists.txt cb23ccb8a9b0421a554b41234c3d9ced3965d378 Diff: https://git.reviewboard.kde.org/r/123742/diff/ Testing --- KNewStuff (and any other framework we add these changes to) now reports the ECM url and name when it isn't found at cmake time. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Kross module loading is broken since KF5.0.0
2015-05-12 14:39 GMT+03:00 Vishesh Handa m...@vhanda.in: On Mon, May 11, 2015 at 6:37 PM, Alexander Potashev aspotas...@gmail.com wrote: 3. Make module names case insensitive. This can be implemented in Kross framework by listing and filtering directory contents instead of checking for a specific file. It will be slower and this looks like a hack to me. I would go for this one. It's trivial to implement and the speed difference would be minuscule. Hi Vishesh and others, You are right it's easy to implement and will not be slow, but a. This puts us in a ridiculous position where we are slaves of naming conventions. Easy-going users will shoot us down in flames. b. I don't like case insensitive names in programming for the same reason identifiers are case sensitive in most programming languages: as soon as there's no rule, anyone is free to spell it forms, then FORMS and finally fOrMs. It seemed like all three solutions are equally bad, but I finally found an excuse to ignore the library naming convention: Kross modules are not C++ libraries, they are _plugins_. Notice that the rule Library names are in CamelCase goes in section Frameworks buildsystem is consistent, thus it is meant to beautify CMakeLists.txt files where you may want to link to KF5 libraries. But nobody will ever link to a Kross plugin bypassing the standard loader for Kross modules. That said, I think it should be absolutely fine if we rename all Kross modules back to lowercase spelling and be happy again. What do you think? -- Alexander Potashev ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123743: Fix missing category to silence `desktop-file-validate`
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123743/ --- (Updated May 13, 2015, 12:50 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit aa3c4e7797940e53433dfbd38a1bdc4b0a5727a6 by Jan Kundrát to branch master. Repository: ksshaskpass Description --- src/org.kde.ksshaskpass.desktop: error: (will be fatal in the future): value Security in key Categories in group Desktop Entry requires another category to be present among the following categories: Settings, or System ...and System sounds closer to the actual purpose of this application. Either this one, or let's remove the Security bit altogether. Diffs - src/org.kde.ksshaskpass.desktop a5fa682b7e6195be2474f2547714d71d68dd7284 Diff: https://git.reviewboard.kde.org/r/123743/diff/ Testing --- Thanks, Jan Kundrát ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123742: knewstuff: Add verbose ecm message when ECM isn't found.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/#review80299 --- CMakeLists.txt (line 8) https://git.reviewboard.kde.org/r/123742/#comment55093 I insist, feature_sumary goes at the bottom. (and is already there) Is it needed to have it twice? - Aleix Pol Gonzalez On May 13, 2015, 3 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/ --- (Updated May 13, 2015, 3 p.m.) Review request for KDE Frameworks and Jeremy Whiting. Repository: knewstuff Description --- Make ECM missing message explain a) what ECM is, and b) where to find it. Diffs - CMakeLists.txt cb23ccb8a9b0421a554b41234c3d9ced3965d378 Diff: https://git.reviewboard.kde.org/r/123742/diff/ Testing --- KNewStuff (and any other framework we add these changes to) now reports the ECM url and name when it isn't found at cmake time. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 123781: Don't mangle UDS_TARGET_URL to UDS_LOCAL_PATH in UDSEntries
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/ --- Review request for KDE Frameworks, David Faure and Fredrik Höglund. Repository: kio-extras Description --- kio_desktop's prepareUDSEntry() implementation currently overwrites the entry's UDS_TARGET_URL with UDS_LOCAL_PATH. Down the line this causes KFileItem to return QUrls with an empty scheme(), which leads to problems in kio/src/widgets/krun.cpp's resolveURLs(), used internally by KRun::runService. resolveURLs() will determine that the app doesn't support the (empty) scheme and fall through to check whether it meets the criteria for a KProtocolInfo::protocolClass of :local (which it doesn't either) before running KIO::mostLocalUrl (which thus isn't reached, but if it were, would also balk on an invalid QUrl). Ultimately the URL isn't getting fixed up, which in the case of using an action produced by KFileItemActions::addOpenWithActionsTo will cause the subprocess to be started non-blocking (freezing plasmashell in Folder View's case) and throw up a Couldn't launch kioexec error dialog box once it exits. This patch simply removes the mangling (originally added by b0f798df), which will cause the entries to have the original desktop:/ URL. When an app doesn't explicitly support this protocol the fallback logic in resolvedURLs() will then produce a file:// URL. This fits in with the overall approach of producing the URLs needed by the app (based on its .desktop file) in KRun, which has all the support it needs to produce local URLs from desktop:/. Double-clicking files in Folder View wasn't affected because it already had a hack to set the scheme for scheme-less URLs to 'file'; this workaround can be dropped once plasma-desktop depends on a KIO version with this patch applied. Diffs - desktop/kio_desktop.cpp 28fdfe4 Diff: https://git.reviewboard.kde.org/r/123781/diff/ Testing --- Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123781: Don't mangle UDS_TARGET_URL to UDS_LOCAL_PATH in UDSEntries
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/ --- (Updated May 13, 2015, 7:38 p.m.) Review request for KDE Frameworks, Plasma, David Faure, and Fredrik Höglund. Changes --- Add testing info. Repository: kio-extras Description --- kio_desktop's prepareUDSEntry() implementation currently overwrites the entry's UDS_TARGET_URL with UDS_LOCAL_PATH. Down the line this causes KFileItem to return QUrls with an empty scheme(), which leads to problems in kio/src/widgets/krun.cpp's resolveURLs(), used internally by KRun::runService. resolveURLs() will determine that the app doesn't support the (empty) scheme and fall through to check whether it meets the criteria for a KProtocolInfo::protocolClass of :local (which it doesn't either) before running KIO::mostLocalUrl (which thus isn't reached, but if it were, would also balk on an invalid QUrl). Ultimately the URL isn't getting fixed up, which in the case of using an action produced by KFileItemActions::addOpenWithActionsTo will cause the subprocess to be started non-blocking (freezing plasmashell in Folder View's case) and throw up a Couldn't launch kioexec error dialog box once it exits. This patch simply removes the mangling (originally added by b0f798df), which will cause the entries to have the original desktop:/ URL. When an app doesn't explicitly support this protocol the fallback logic in resolvedURLs() will then produce a file:// URL. This fits in with the overall approach of producing the URLs needed by the app (based on its .desktop file) in KRun, which has all the support it needs to produce local URLs from desktop:/. Double-clicking files in Folder View wasn't affected because it already had a hack to set the scheme for scheme-less URLs to 'file'; this workaround can be dropped once plasma-desktop depends on a KIO version with this patch applied. Diffs - desktop/kio_desktop.cpp 28fdfe4 Diff: https://git.reviewboard.kde.org/r/123781/diff/ Testing (updated) --- Tried various KDE and non-KDE apps. Also compared this to the URLs handed to KRun by the regular local file browsing slave; they also use the file scheme. Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Install location of myframework_version.h headers
On Wednesday 13 May 2015 08:37:19 Kevin Funk wrote:
Right, and I just noticed a problem I didn't think of before: There's no
$PREFIX/include/KF5/$myframework/ where I could install
${myframework}_version.h into for some of the frameworks :)
KConfig for instance has two libs, but only installs a single version
header. - $PREFIX/include/KF5/KConfigCore/...
- $PREFIX/include/KF5/KConfigGui/...
- $PREFIX/include/KF5/kconfig_version.h
Now in order to move kconfig_version.h to a proper location, I'd have to
install both a kconfigcore_version.h and kconfiggui_version.h to their
resp. locations:
- $PREFIX/include/KF5/KConfigCore/kconfigcore_version.h
- $PREFIX/include/KF5/KConfigGui/kconfiggui_version.h
This would make the CMake code for installing the version header quite
different for those modules, so I'm wondering how to proceed...
A possible solution is to install kconfig_version.h to
$PREFIX/include/KF5/KConfig, and add that to the exported include paths for
both libraries.
Alex
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123439: Coding Style for karchive_p.h kbzip2filter.h kcompressiondevice.h kfilterbase.h kfilterdev.h klimitediodevice_p.h kxzfilter.h kzip.h
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123439/ --- (Updated May 14, 2015, 1:08 a.m.) Review request for Akonadi, KDE Frameworks, Mario Bensi, and David Faure. Repository: karchive Description --- We use for karchive the same rules as for kdepim: http://techbase.kde.org/Policies/Kdepim_Coding_Style If wished, we could have extra policy for karchive. (Let me know) Diffs - src/karchive_p.h 26a98659b860b379f28ee9442df7a0dd5209f89d src/kbzip2filter.h 767dabe src/kcompressiondevice.h 9e0c2c0f04ac9f4401e9f1a8c2a0b71f3ef327ed src/kfilterbase.h 9cbaf950ce42878d426b358582c3b62a1ded9aaf src/klimitediodevice_p.h 51929aa src/kxzfilter.h 8747a29 src/kzip.h 11ca330 Diff: https://git.reviewboard.kde.org/r/123439/diff/ Testing --- Could anybody take care of this review after 30 days? Thanks. Thanks, Guy Maurel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 123784: Make sure we're not magically converting from QString to QUrl
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123784/ --- Review request for KDE Frameworks and Plasma. Repository: kio-extras Description --- Spots a couple of bugs and lets us all rejoice for using a type-safe language (when the correct flags are set). Diffs - CMakeLists.txt bfc97ad desktop/desktopnotifier.cpp af43f84 filenamesearch/kio_filenamesearch.cpp 7d59b75 mtp/kio_mtp.cpp 6487def network/ioslave/networkslave.cpp 0e90c3a network/kded/kioslavenotifier.cpp 77fe22c sftp/kio_sftp.cpp 8e9ab37 Diff: https://git.reviewboard.kde.org/r/123784/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 123784: Make sure we're not magically converting from QString to QUrl
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123784/#review80324 --- Ship it! Ship It! - Eike Hein On May 14, 2015, 12:02 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123784/ --- (Updated May 14, 2015, 12:02 a.m.) Review request for KDE Frameworks and Plasma. Repository: kio-extras Description --- Spots a couple of bugs and lets us all rejoice for using a type-safe language (when the correct flags are set). Diffs - CMakeLists.txt bfc97ad desktop/desktopnotifier.cpp af43f84 filenamesearch/kio_filenamesearch.cpp 7d59b75 mtp/kio_mtp.cpp 6487def network/ioslave/networkslave.cpp 0e90c3a network/kded/kioslavenotifier.cpp 77fe22c sftp/kio_sftp.cpp 8e9ab37 Diff: https://git.reviewboard.kde.org/r/123784/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 123781: Don't mangle UDS_TARGET_URL to UDS_LOCAL_PATH in UDSEntries
On May 13, 2015, 11:09 p.m., Aleix Pol Gonzalez wrote: I'm not against the patch, but in general urls without scheme Aleix Pol Gonzalez wrote: Eh sorry, I published before finishing the sentence. I'm not against the patch, but in general urls without scheme usually sound like QUrl API misuse. Eike Hein wrote: If you're asking why KFileItem ends up returning a QUrl without a scheme after UDS_TARGET_URL is set to UDS_LOCAL_PATH: Because QUrl(/path/to/bla).scheme().isEmpty() is true. Aleix Pol Gonzalez wrote: True, so can we turn the QUrl(/path/to/bla) into QUrl::fromLocalFile(/path/to/bla)? No, because it makes no sense to run fromLocalFile() over the UDS_TARGET_URL field, since it's not meant to be a local path. A KFileItem isn't required to be a local file (that's why it has a isLocalFile()). kio_desktop was wrong to put the local path into the field. I assume it was a hack designed to make sure consumers of the slave get local paths, but as explained KRun contains logic to resolve desktop:/ to a file:// URL when needed by the app already. - Eike --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/#review80321 --- On May 13, 2015, 7:38 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/ --- (Updated May 13, 2015, 7:38 p.m.) Review request for KDE Frameworks, Plasma, David Faure, and Fredrik Höglund. Repository: kio-extras Description --- kio_desktop's prepareUDSEntry() implementation currently overwrites the entry's UDS_TARGET_URL with UDS_LOCAL_PATH. Down the line this causes KFileItem to return QUrls with an empty scheme(), which leads to problems in kio/src/widgets/krun.cpp's resolveURLs(), used internally by KRun::runService. resolveURLs() will determine that the app doesn't support the (empty) scheme and fall through to check whether it meets the criteria for a KProtocolInfo::protocolClass of :local (which it doesn't either) before running KIO::mostLocalUrl (which thus isn't reached, but if it were, would also balk on an invalid QUrl). Ultimately the URL isn't getting fixed up, which in the case of using an action produced by KFileItemActions::addOpenWithActionsTo will cause the subprocess to be started non-blocking (freezing plasmashell in Folder View's case) and throw up a Couldn't launch kioexec error dialog box once it exits. This patch simply removes the mangling (originally added by b0f798df), which will cause the entries to have the original desktop:/ URL. When an app doesn't explicitly support this protocol the fallback logic in resolvedURLs() will then produce a file:// URL. This fits in with the overall approach of producing the URLs needed by the app (based on its .desktop file) in KRun, which has all the support it needs to produce local URLs from desktop:/. Double-clicking files in Folder View wasn't affected because it already had a hack to set the scheme for scheme-less URLs to 'file'; this workaround can be dropped once plasma-desktop depends on a KIO version with this patch applied. Diffs - desktop/kio_desktop.cpp 28fdfe4 Diff: https://git.reviewboard.kde.org/r/123781/diff/ Testing --- Tried various KDE and non-KDE apps. Also compared this to the URLs handed to KRun by the regular local file browsing slave; they also use the file scheme. Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123781: Don't mangle UDS_TARGET_URL to UDS_LOCAL_PATH in UDSEntries
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/#review80321 --- I'm not against the patch, but in general urls without scheme - Aleix Pol Gonzalez On May 13, 2015, 9:38 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/ --- (Updated May 13, 2015, 9:38 p.m.) Review request for KDE Frameworks, Plasma, David Faure, and Fredrik Höglund. Repository: kio-extras Description --- kio_desktop's prepareUDSEntry() implementation currently overwrites the entry's UDS_TARGET_URL with UDS_LOCAL_PATH. Down the line this causes KFileItem to return QUrls with an empty scheme(), which leads to problems in kio/src/widgets/krun.cpp's resolveURLs(), used internally by KRun::runService. resolveURLs() will determine that the app doesn't support the (empty) scheme and fall through to check whether it meets the criteria for a KProtocolInfo::protocolClass of :local (which it doesn't either) before running KIO::mostLocalUrl (which thus isn't reached, but if it were, would also balk on an invalid QUrl). Ultimately the URL isn't getting fixed up, which in the case of using an action produced by KFileItemActions::addOpenWithActionsTo will cause the subprocess to be started non-blocking (freezing plasmashell in Folder View's case) and throw up a Couldn't launch kioexec error dialog box once it exits. This patch simply removes the mangling (originally added by b0f798df), which will cause the entries to have the original desktop:/ URL. When an app doesn't explicitly support this protocol the fallback logic in resolvedURLs() will then produce a file:// URL. This fits in with the overall approach of producing the URLs needed by the app (based on its .desktop file) in KRun, which has all the support it needs to produce local URLs from desktop:/. Double-clicking files in Folder View wasn't affected because it already had a hack to set the scheme for scheme-less URLs to 'file'; this workaround can be dropped once plasma-desktop depends on a KIO version with this patch applied. Diffs - desktop/kio_desktop.cpp 28fdfe4 Diff: https://git.reviewboard.kde.org/r/123781/diff/ Testing --- Tried various KDE and non-KDE apps. Also compared this to the URLs handed to KRun by the regular local file browsing slave; they also use the file scheme. Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123781: Don't mangle UDS_TARGET_URL to UDS_LOCAL_PATH in UDSEntries
On May 14, 2015, 1:09 a.m., Aleix Pol Gonzalez wrote: I'm not against the patch, but in general urls without scheme Eh sorry, I published before finishing the sentence. I'm not against the patch, but in general urls without scheme usually sound like QUrl API misuse. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/#review80321 --- On May 13, 2015, 9:38 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/ --- (Updated May 13, 2015, 9:38 p.m.) Review request for KDE Frameworks, Plasma, David Faure, and Fredrik Höglund. Repository: kio-extras Description --- kio_desktop's prepareUDSEntry() implementation currently overwrites the entry's UDS_TARGET_URL with UDS_LOCAL_PATH. Down the line this causes KFileItem to return QUrls with an empty scheme(), which leads to problems in kio/src/widgets/krun.cpp's resolveURLs(), used internally by KRun::runService. resolveURLs() will determine that the app doesn't support the (empty) scheme and fall through to check whether it meets the criteria for a KProtocolInfo::protocolClass of :local (which it doesn't either) before running KIO::mostLocalUrl (which thus isn't reached, but if it were, would also balk on an invalid QUrl). Ultimately the URL isn't getting fixed up, which in the case of using an action produced by KFileItemActions::addOpenWithActionsTo will cause the subprocess to be started non-blocking (freezing plasmashell in Folder View's case) and throw up a Couldn't launch kioexec error dialog box once it exits. This patch simply removes the mangling (originally added by b0f798df), which will cause the entries to have the original desktop:/ URL. When an app doesn't explicitly support this protocol the fallback logic in resolvedURLs() will then produce a file:// URL. This fits in with the overall approach of producing the URLs needed by the app (based on its .desktop file) in KRun, which has all the support it needs to produce local URLs from desktop:/. Double-clicking files in Folder View wasn't affected because it already had a hack to set the scheme for scheme-less URLs to 'file'; this workaround can be dropped once plasma-desktop depends on a KIO version with this patch applied. Diffs - desktop/kio_desktop.cpp 28fdfe4 Diff: https://git.reviewboard.kde.org/r/123781/diff/ Testing --- Tried various KDE and non-KDE apps. Also compared this to the URLs handed to KRun by the regular local file browsing slave; they also use the file scheme. Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123781: Don't mangle UDS_TARGET_URL to UDS_LOCAL_PATH in UDSEntries
On May 13, 2015, 11:09 p.m., Aleix Pol Gonzalez wrote: I'm not against the patch, but in general urls without scheme Aleix Pol Gonzalez wrote: Eh sorry, I published before finishing the sentence. I'm not against the patch, but in general urls without scheme usually sound like QUrl API misuse. If you're asking why KFileItem ends up returning a QUrl without a scheme after UDS_TARGET_URL is set to UDS_LOCAL_PATH: Because QUrl(/path/to/bla).scheme().isEmpty() is true. - Eike --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/#review80321 --- On May 13, 2015, 7:38 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/ --- (Updated May 13, 2015, 7:38 p.m.) Review request for KDE Frameworks, Plasma, David Faure, and Fredrik Höglund. Repository: kio-extras Description --- kio_desktop's prepareUDSEntry() implementation currently overwrites the entry's UDS_TARGET_URL with UDS_LOCAL_PATH. Down the line this causes KFileItem to return QUrls with an empty scheme(), which leads to problems in kio/src/widgets/krun.cpp's resolveURLs(), used internally by KRun::runService. resolveURLs() will determine that the app doesn't support the (empty) scheme and fall through to check whether it meets the criteria for a KProtocolInfo::protocolClass of :local (which it doesn't either) before running KIO::mostLocalUrl (which thus isn't reached, but if it were, would also balk on an invalid QUrl). Ultimately the URL isn't getting fixed up, which in the case of using an action produced by KFileItemActions::addOpenWithActionsTo will cause the subprocess to be started non-blocking (freezing plasmashell in Folder View's case) and throw up a Couldn't launch kioexec error dialog box once it exits. This patch simply removes the mangling (originally added by b0f798df), which will cause the entries to have the original desktop:/ URL. When an app doesn't explicitly support this protocol the fallback logic in resolvedURLs() will then produce a file:// URL. This fits in with the overall approach of producing the URLs needed by the app (based on its .desktop file) in KRun, which has all the support it needs to produce local URLs from desktop:/. Double-clicking files in Folder View wasn't affected because it already had a hack to set the scheme for scheme-less URLs to 'file'; this workaround can be dropped once plasma-desktop depends on a KIO version with this patch applied. Diffs - desktop/kio_desktop.cpp 28fdfe4 Diff: https://git.reviewboard.kde.org/r/123781/diff/ Testing --- Tried various KDE and non-KDE apps. Also compared this to the URLs handed to KRun by the regular local file browsing slave; they also use the file scheme. Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123781: Don't mangle UDS_TARGET_URL to UDS_LOCAL_PATH in UDSEntries
On May 14, 2015, 1:09 a.m., Aleix Pol Gonzalez wrote: I'm not against the patch, but in general urls without scheme Aleix Pol Gonzalez wrote: Eh sorry, I published before finishing the sentence. I'm not against the patch, but in general urls without scheme usually sound like QUrl API misuse. Eike Hein wrote: If you're asking why KFileItem ends up returning a QUrl without a scheme after UDS_TARGET_URL is set to UDS_LOCAL_PATH: Because QUrl(/path/to/bla).scheme().isEmpty() is true. True, so can we turn the QUrl(/path/to/bla) into QUrl::fromLocalFile(/path/to/bla)? - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/#review80321 --- On May 13, 2015, 9:38 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123781/ --- (Updated May 13, 2015, 9:38 p.m.) Review request for KDE Frameworks, Plasma, David Faure, and Fredrik Höglund. Repository: kio-extras Description --- kio_desktop's prepareUDSEntry() implementation currently overwrites the entry's UDS_TARGET_URL with UDS_LOCAL_PATH. Down the line this causes KFileItem to return QUrls with an empty scheme(), which leads to problems in kio/src/widgets/krun.cpp's resolveURLs(), used internally by KRun::runService. resolveURLs() will determine that the app doesn't support the (empty) scheme and fall through to check whether it meets the criteria for a KProtocolInfo::protocolClass of :local (which it doesn't either) before running KIO::mostLocalUrl (which thus isn't reached, but if it were, would also balk on an invalid QUrl). Ultimately the URL isn't getting fixed up, which in the case of using an action produced by KFileItemActions::addOpenWithActionsTo will cause the subprocess to be started non-blocking (freezing plasmashell in Folder View's case) and throw up a Couldn't launch kioexec error dialog box once it exits. This patch simply removes the mangling (originally added by b0f798df), which will cause the entries to have the original desktop:/ URL. When an app doesn't explicitly support this protocol the fallback logic in resolvedURLs() will then produce a file:// URL. This fits in with the overall approach of producing the URLs needed by the app (based on its .desktop file) in KRun, which has all the support it needs to produce local URLs from desktop:/. Double-clicking files in Folder View wasn't affected because it already had a hack to set the scheme for scheme-less URLs to 'file'; this workaround can be dropped once plasma-desktop depends on a KIO version with this patch applied. Diffs - desktop/kio_desktop.cpp 28fdfe4 Diff: https://git.reviewboard.kde.org/r/123781/diff/ Testing --- Tried various KDE and non-KDE apps. Also compared this to the URLs handed to KRun by the regular local file browsing slave; they also use the file scheme. Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123784: Make sure we're not magically converting from QString to QUrl
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123784/ --- (Updated May 14, 2015, 12:21 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Changes --- Submitted with commit 0ac89dd7b18138f40b3104074dc423c5329d4ed3 by Aleix Pol to branch master. Repository: kio-extras Description --- Spots a couple of bugs and lets us all rejoice for using a type-safe language (when the correct flags are set). Diffs - CMakeLists.txt bfc97ad desktop/desktopnotifier.cpp af43f84 filenamesearch/kio_filenamesearch.cpp 7d59b75 mtp/kio_mtp.cpp 6487def network/ioslave/networkslave.cpp 0e90c3a network/kded/kioslavenotifier.cpp 77fe22c sftp/kio_sftp.cpp 8e9ab37 Diff: https://git.reviewboard.kde.org/r/123784/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 123778: Fix crash when DBus call ends with error
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123778/ --- Review request for KDE Frameworks and Ivan Čukić. Repository: kactivities Description --- If the DBus call ends with error, reportResult is not called in DBusCallFutureInterface and calling QFuture::result() without actually setting the result leads to crash. In case of error, continuation callback is not called (maybe it would be good to somehow indicate the error?). Diffs - src/utils/dbusfuture_p.h 336235c Diff: https://git.reviewboard.kde.org/r/123778/diff/ Testing --- No longer crashes Thanks, David Rosca ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123742: knewstuff: Add verbose ecm message when ECM isn't found.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/ --- (Updated May 14, 2015, 12:47 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Jeremy Whiting. Changes --- Submitted with commit 01c3f36cf657e0e7e68631e14f884de9a3b3ebcd by Jeremy Whiting to branch master. Repository: knewstuff Description --- Make ECM missing message explain a) what ECM is, and b) where to find it. Diffs - CMakeLists.txt cb23ccb8a9b0421a554b41234c3d9ced3965d378 Diff: https://git.reviewboard.kde.org/r/123742/diff/ Testing --- KNewStuff (and any other framework we add these changes to) now reports the ECM url and name when it isn't found at cmake time. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123575: Improve debug information for some plasmoids
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123575/ --- (Updated May 14, 2015, 12:57 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Plasma and Marco Martin. Changes --- Submitted with commit dedc3cc9ae6e7af680ffc894d2f1b9585973fc65 by Aleix Pol on behalf of Aleix Pol Gonzalez to branch master. Repository: plasma-framework Description --- I've been trying to figure out why there's so many qml scripts which errors always specify Unknown Files. This patch is not ideal, but improves the situation slightly. Diffs - src/plasmaquick/appletquickitem.cpp 0748a8d Diff: https://git.reviewboard.kde.org/r/123575/diff/ Testing --- * Tests still pass. * some warnings have a filename when running plasmawindowed org.kde.plasma.systemtray. 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 123737: Use QTemporaryFile instead of hardcoding a temporary file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123737/#review80313 --- Ship it! Ship It! - Aleix Pol Gonzalez On May 13, 2015, 7:13 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123737/ --- (Updated May 13, 2015, 7:13 p.m.) Review request for KDE Frameworks. Repository: kdelibs4support Description --- Hardcoding files like this seems like a bad idea. Also updates the source URL to something fetchable. This also affects kdelibs. Diffs - tests/netaccesstest.cpp a06b49d015fe420fd0293292041caa988422f521 Diff: https://git.reviewboard.kde.org/r/123737/diff/ Testing --- Test still passes on Linux. Can't test on Windows. 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 123735: version of QmlObject with a static engine
On May 12, 2015, 5:45 p.m., Mark Gaiser wrote: src/kdeclarative/qmlobjectsharedengine.h, line 57 https://git.reviewboard.kde.org/r/123735/diff/5/?file=368396#file368396line57 std::unique_ptr... ... then you can also forget about the delete in the destructor. buh, fine On May 12, 2015, 5:45 p.m., Mark Gaiser wrote: src/kdeclarative/qmlobjectsharedengine.cpp, line 48 https://git.reviewboard.kde.org/r/123735/diff/5/?file=368397#file368397line48 static const QQmlEngine ... Prevents changing the pointer later on. not at the moment, since it's initialized to 0 and then instantiated only if needed, and refcounted. it may become const if it would get instantiated no matter what, but don't like it that much - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123735/#review80252 --- On May 12, 2015, 4:12 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123735/ --- (Updated May 12, 2015, 4:12 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- to make easier doing applications like plasma that use a lot of qml to have a single engine make a subclass of QmlObject called QmlObjectSharedEngine that has a single, static QQmlEngine Diffs - src/kdeclarative/CMakeLists.txt d73bff0 src/kdeclarative/kdeclarative.cpp b3906e2 src/kdeclarative/qmlobject.h f26b67d src/kdeclarative/qmlobject.cpp c483665 src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123735/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123737: Use QTemporaryFile instead of hardcoding a temporary file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123737/ --- (Updated May 13, 2015, 5:26 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit b87835cc7bdf869624b77fea7e6b7b0526c33ec6 by Michael Palimaka to branch master. Repository: kdelibs4support Description --- Hardcoding files like this seems like a bad idea. Also updates the source URL to something fetchable. This also affects kdelibs. Diffs - tests/netaccesstest.cpp a06b49d015fe420fd0293292041caa988422f521 Diff: https://git.reviewboard.kde.org/r/123737/diff/ Testing --- Test still passes on Linux. Can't test on Windows. 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 123735: version of QmlObject with a static engine
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123735/ --- (Updated May 13, 2015, 5:37 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- to make easier doing applications like plasma that use a lot of qml to have a single engine make a subclass of QmlObject called QmlObjectSharedEngine that has a single, static QQmlEngine Diffs (updated) - src/kdeclarative/CMakeLists.txt d73bff0 src/kdeclarative/kdeclarative.cpp b3906e2 src/kdeclarative/qmlobject.h f26b67d src/kdeclarative/qmlobject.cpp c483665 src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123735/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123737: Use QTemporaryFile instead of hardcoding a temporary file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123737/ --- (Updated May 13, 2015, 5:13 p.m.) Review request for KDE Frameworks. Changes --- Eliminate ifdefs. Repository: kdelibs4support Description --- Hardcoding files like this seems like a bad idea. Also updates the source URL to something fetchable. This also affects kdelibs. Diffs (updated) - tests/netaccesstest.cpp a06b49d015fe420fd0293292041caa988422f521 Diff: https://git.reviewboard.kde.org/r/123737/diff/ Testing --- Test still passes on Linux. Can't test on Windows. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Install location of myframework_version.h headers
On Monday 04 May 2015 14:11:14 Kevin Funk wrote:
On Monday 04 May 2015 13:11:05 David Faure wrote:
On Monday 04 May 2015 12:29:39 David Faure wrote:
On Monday 04 May 2015 12:23:32 Kevin Funk wrote:
The issue could be easily fixed by moving the myframework_version.h
into
$PREFIX/include/KF5/$FRAMEWORK/, no?
Correct, this is what KArchive does.
Ooops, I thought we were talking about the generated export header.
You are right, all the version headers are in include/KF5 directly.
I think this is an oversight. Well, I did see it and didn't think it would
be a problem, but I think the only real reason I didn't move that one when
moving all other headers under $FRAMEWORK is that version headers are
installed by the toplevel CMakeLists.txt while I was working on the
CMakeLists.txt for the libs themselves.
Right, and I just noticed a problem I didn't think of before: There's no
$PREFIX/include/KF5/$myframework/ where I could install
${myframework}_version.h into for some of the frameworks :)
KConfig for instance has two libs, but only installs a single version header.
- $PREFIX/include/KF5/KConfigCore/...
- $PREFIX/include/KF5/KConfigGui/...
- $PREFIX/include/KF5/kconfig_version.h
Now in order to move kconfig_version.h to a proper location, I'd have to
install both a kconfigcore_version.h and kconfiggui_version.h to their resp.
locations:
- $PREFIX/include/KF5/KConfigCore/kconfigcore_version.h
- $PREFIX/include/KF5/KConfigGui/kconfiggui_version.h
This would make the CMake code for installing the version header quite
different for those modules, so I'm wondering how to proceed...
Cheers
I would be OK with *all* framework_version.h headers being installed under
include/KF5/$FRAMEWORK. I.e. change them all, not just one framework.
Sure.
This is SC since apps using the framework do get the include path
automatically. The only exception I can think of would be if some cmake
code was trying to locate (and possibly parse) the version.h header, but
that sounds convoluted given that there are cmake variables with the
version numbers in them already, much much more convenient to use.
In any case, do a full build of everything kf5 based (with a clean kf5
install dir) to make sure nothing breaks :-)
Alright, thanks for the insight!
Will post a review-request as soon as possible.
Thanks
Hm, just gave this a try but then stumbled upon KConfig...
There are two modules
--
Kevin Funk | kf...@kde.org | http://kfunk.org
signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123742: knewstuff: Add verbose ecm message when ECM isn't found.
On May 13, 2015, 6:45 a.m., Kevin Funk wrote: I'd reorder the code: ``` ... include(FeatureSummary) find_package(ECM ...) set_target_properties(ECM ...) feature_summary(...) ... ``` I know that it is a bit harder to script this way , but helps code readability :D Just to say, +1 from me. Better error reporting is always helpful. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/#review80275 --- On May 13, 2015, 12:28 a.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/ --- (Updated May 13, 2015, 12:28 a.m.) Review request for KDE Frameworks and Jeremy Whiting. Repository: knewstuff Description --- Make ECM missing message explain a) what ECM is, and b) where to find it. Diffs - CMakeLists.txt cb23ccb8a9b0421a554b41234c3d9ced3965d378 Diff: https://git.reviewboard.kde.org/r/123742/diff/ Testing --- KNewStuff (and any other framework we add these changes to) now reports the ECM url and name when it isn't found at cmake time. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123742: knewstuff: Add verbose ecm message when ECM isn't found.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/#review80275 --- I'd reorder the code: ``` ... include(FeatureSummary) find_package(ECM ...) set_target_properties(ECM ...) feature_summary(...) ... ``` I know that it is a bit harder to script this way , but helps code readability :D - Kevin Funk On May 13, 2015, 12:28 a.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123742/ --- (Updated May 13, 2015, 12:28 a.m.) Review request for KDE Frameworks and Jeremy Whiting. Repository: knewstuff Description --- Make ECM missing message explain a) what ECM is, and b) where to find it. Diffs - CMakeLists.txt cb23ccb8a9b0421a554b41234c3d9ced3965d378 Diff: https://git.reviewboard.kde.org/r/123742/diff/ Testing --- KNewStuff (and any other framework we add these changes to) now reports the ECM url and name when it isn't found at cmake time. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123745: Stop producing warnings about CMP0037 policy
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123745/
---
(Updated May 13, 2015, 8:02 p.m.)
Status
--
This change has been marked as submitted.
Review request for KDE Frameworks and Aleix Pol Gonzalez.
Changes
---
Submitted with commit 38b0eeec41de0e916dd834b849f7ddcfa06bdafc by Hrvoje Senjan
to branch master.
Repository: kauth
Description
---
Let cmake know ALL is not the name in this custom command. As a bonus, one can
really execute make ${HELPER_ID}.policy in projects where policies are
generated by kauth.
Diffs
-
cmake/KF5AuthMacros.cmake 3508236
Diff: https://git.reviewboard.kde.org/r/123745/diff/
Testing
---
Configured powerdevil, no warning, before it nagged:
```
[ 110s] CMake Warning (dev) at /usr/lib64/cmake/KF5Auth/KF5AuthMacros.cmake:76
(add_custom_target):
[ 110s] Policy CMP0037 is not set: Target names should not be reserved and
should
[ 110s] match a validity pattern. Run cmake --help-policy CMP0037 for
policy
[ 110s] details. Use the cmake_policy command to set the policy and
suppress this
[ 110s] warning.
[ 110s]
[ 110s] The target name actions for org.kde.powerdevil.backlighthelper is
[ 110s] reserved or not valid for certain CMake features, such as generator
[ 110s] expressions, and may result in undefined behavior.
[ 110s] Call Stack (most recent call first):
[ 110s] daemon/BackendConfig.cmake:46 (kauth_install_actions)
[ 110s] daemon/CMakeLists.txt:96 (include)
```
Also make org.kde.powerdevil.backlighthelper.policy now works.
Thanks,
Hrvoje Senjan
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
