Re: Review Request 119323: fix auth race condition
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/#review62631 --- Ship it! I have been using this for 4.x and 5.x for a while without issues. Unit tests (no matter how little they are) also pass, so this should go in. IMO this should also be pushed for kdelibs, which is affected in the same manner. - Luca Beltrame On Lug. 17, 2014, 11:23 a.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/ --- (Updated Lug. 17, 2014, 11:23 a.m.) Review request for KDE Frameworks, Hrvoje Senjan, Luca Beltrame, and Martin Bříza. Repository: kauth Description --- pid based auth is racy because of pid reuse, don't use it. Diffs - src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/119323/diff/ Testing --- it builds Thanks, Martin Tobias Holmedahl Sandsmark ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119331: Use CMAKE_INSTALL_FULL_LIBEXECDIR_KF5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119331/#review62636 --- Ship it! Ship It! - Alex Merry On July 17, 2014, 11:01 a.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119331/ --- (Updated July 17, 2014, 11:01 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Resolves the problem of passing relative vs. absolute KF5_LIBEXEC_INSTALL_DIR/LIBEXEC_INSTALL_DIR. Diffs - autotests/CMakeLists.txt a9075e3 autotests/krununittest.cpp 9bbceb2 src/core/config-kiocore.h.cmake 6041c9d src/core/desktopexecparser.cpp be62791 src/core/slave.cpp c842f01 src/ioslaves/http/config-kioslave-http.h.cmake 3f313e9 src/ioslaves/http/http.cpp 55a19f4 src/kpac/config-kpac.h.cmake 1e73657 src/kpac/discovery.cpp d108fee Diff: https://git.reviewboard.kde.org/r/119331/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 119345: Port away from deprecated QUrl API
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119345/ --- (Updated July 18, 2014, 9:34 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: attica Description --- This means we no longer need to set QT_DISABLE_DEPRECATED_BEFORE=0 Diffs - src/CMakeLists.txt 8b8ecad61629ad5fdf23b1628587bf4a5d109e65 src/provider.cpp 40ee8876e7fc5f6eae322254f074110c321a33fc Diff: https://git.reviewboard.kde.org/r/119345/diff/ Testing --- Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119349: typo in signal names ( upowermanager )
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119349/#review62638 --- Hmm, my upower (and also the newer one) has DeviceAdded/Removed signals... how did you come to this conclusion? Have you done any testing? - Lukáš Tinkl On Čec. 18, 2014, 7:31 dop., Ömer Fadıl Usta wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119349/ --- (Updated Čec. 18, 2014, 7:31 dop.) Review request for KDE Frameworks and Solid. Repository: solid Description --- There were some typos for signals in upowermanager such as ; DeviceAdded -- deviceAdded DeviceRemoved -- deviceRemoved Diffs - src/solid/devices/backends/upower/upowermanager.cpp 53c8580 Diff: https://git.reviewboard.kde.org/r/119349/diff/ Testing --- Thanks, Ömer Fadıl Usta ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119349: typo in signal names ( upowermanager )
On July 18, 2014, 12:50 p.m., Lukáš Tinkl wrote: Hmm, my upower (and also the newer one) has DeviceAdded/Removed signals... how did you come to this conclusion? Have you done any testing? Yes you are right, after making test the result is same. Still getting no such Signal erros in my .xsesssion-errors file. I will discard this patch but there is a problem about that signal - Ömer Fadıl --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119349/#review62638 --- On July 18, 2014, 8:31 a.m., Ömer Fadıl Usta wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119349/ --- (Updated July 18, 2014, 8:31 a.m.) Review request for KDE Frameworks and Solid. Repository: solid Description --- There were some typos for signals in upowermanager such as ; DeviceAdded -- deviceAdded DeviceRemoved -- deviceRemoved Diffs - src/solid/devices/backends/upower/upowermanager.cpp 53c8580 Diff: https://git.reviewboard.kde.org/r/119349/diff/ Testing --- Thanks, Ömer Fadıl Usta ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119349: typo in signal names ( upowermanager )
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119349/ --- (Updated July 18, 2014, 12:58 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks and Solid. Repository: solid Description --- There were some typos for signals in upowermanager such as ; DeviceAdded -- deviceAdded DeviceRemoved -- deviceRemoved Diffs - src/solid/devices/backends/upower/upowermanager.cpp 53c8580 Diff: https://git.reviewboard.kde.org/r/119349/diff/ Testing --- Thanks, Ömer Fadıl Usta ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119336: Convert FrameSvg to 9 textures: different approach
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119336/#review62619 --- src/declarativeimports/core/framesvgitem.cpp https://git.reviewboard.kde.org/r/119336/#comment43416 1) This leaks. if you remove a node from a parent you have to delete it yourself. 2) You're calling this from something which is looping through childCount(),so you'll end up either crashing or skipping items. - David Edmundson On July 17, 2014, 8:17 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119336/ --- (Updated July 17, 2014, 8:17 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a derivative of https://git.reviewboard.kde.org/r/119330/ It has a much simpler codebase and it doesn't touch framesvg in the library (and doesn't make things public) the important differences are two, that are imo 100% necessary to maintain a pixel perfect rendering (sacrificing that is a regression simply not acceptable in any case, even for non default themes, ever). At the same time avoids duplication of framesvg code in framesvgitem. potential issues: * e037203748 support for tiling still need porting * yes, it uses a qpainter, that means another copy: but again is necessary as framesvg knows how to render the end result pixel perfect, since for many themes the end piece is *not* the simple rendered element id. * yes, the final scaled texture is still uploaded as a whole, it only avoids to do it too often (like in animations) with event compression, again, only way to be sure it's correctly rendered all the time. The latter two points can have the following optimizations: Iff the frame does not have an overlay and does not have composeoverborders set, the following can be done: * use Svg::image() to fetch the pixmap of the piece, since in that case would be valid * resize the framesvg one single time, at a fixed size , like something 256x256 (256 is not random thing, since we have 8 bits per channel, usually gradients will have no more than 256 stops) and disable the resize timer, this way we are even sure that only one image per prefix will be stored in cache I should add regarding the last two optimization points: i would like to see some real benchmarks about them, or i would not consider then necessary until then. Diffs - tests/dialog.qml PRE-CREATION tests/testborders.qml PRE-CREATION examples/applets/widgetgallery/contents/ui/Buttons.qml 379585f examples/applets/widgetgallery/contents/ui/Menu.qml 1336c42 examples/applets/widgetgallery/contents/ui/standalonemain.qml PRE-CREATION src/declarativeimports/core/framesvgitem.h e155f6a src/declarativeimports/core/framesvgitem.cpp 8320212 src/declarativeimports/core/svgitem.cpp 1ed0631 Diff: https://git.reviewboard.kde.org/r/119336/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119356: Create a QtCore only desktoptojson exe based on the one from kservice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119356/ --- Review request for KDE Frameworks. Repository: kcoreaddons Description --- Unlike the original KConfig based tool it now includes all translations for keys and not just the one for the current locale. Also added a unit test for it. Add a verbose option to desktoptojson Also check for missing entries in the desktoptojson unit test make the desktoptojson unit test less verbose install a KF5CoreAddonsMacros.cmake with kcoreaddons_desktop_to_json() Convert the .desktop files to a new .json format and adapt tests This format is used by KPluginMetaData and allows removing all the useless X-KDE-PluginInfo prefixes Diffs - src/desktoptojson/main.cpp PRE-CREATION CMakeLists.txt d45309f7f5e84c59b2f4d0bf3de68b330d782102 KF5CoreAddonsConfig.cmake.in c471006ee2c8f52b5c22c5edc617554f671237d1 autotests/CMakeLists.txt 75d12932b36fcfe4ae1d538176ef9f85f60f15dd KF5CoreAddonsMacros.cmake PRE-CREATION src/desktoptojson/desktoptojson.cpp PRE-CREATION src/desktoptojson/desktoptojson.h PRE-CREATION src/desktoptojson/CMakeLists.txt PRE-CREATION src/CMakeLists.txt ef1eea63c92532eea7003c67b59bb3649bc02484 autotests/desktoptojsontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119356/diff/ Testing --- Unit test works and passes. 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 119079: Add utility function for loading all plugins from a given dir + easy accessor for metadata
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119079/ --- (Updated Juli 18, 2014, 3:34 nachm.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- This class simplifies reading the metadata from a qt plugin by providing type safe accessor functions for the standard plugininfo keys that are also used by the .desktop file based KPluginInfo KPluginMetaData: Read the translated value for name and description The Name and Comment fields of the metadata should be translated since they will be shown to the user (e.g. in the plugin selection dialog) Add a unit test for KPluginMetaData Add KPluginMetaData::findPlugins() Add a unit test for KPluginMetaData::findPlugins() Introduce KPluginLoader::instantiatePlugins() and add a unit test This method allows easily instantiating all plugins in a given directory KPluginMetaData::pluginName() was changed to return the base name of the plugin file if no plugin name was set in the JSON metadata Diffs (updated) - autotests/CMakeLists.txt 75d12932b36fcfe4ae1d538176ef9f85f60f15dd autotests/jsonplugin.json d86fad49e5d074762d70282b3ace4bf3e6db58df autotests/kpluginloadertest.cpp c8225c02de3a64cae29d88954700dbc6f03ff1b0 autotests/kpluginmetadatatest.cpp PRE-CREATION src/lib/CMakeLists.txt 26eb5a1d4d56742a3395ba2645290bea15aee181 src/lib/plugin/kpluginloader.h 0b7a53d3b879cec1d755b849d9d8c640d251a379 src/lib/plugin/kpluginloader.cpp 9b3c5b6aec537b03b0d8341b33f6f4d7a76c8344 src/lib/plugin/kpluginmetadata.h PRE-CREATION src/lib/plugin/kpluginmetadata.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119079/diff/ Testing --- Added a unit test Should easily allow loading all plugins from a given directory without needing kbuildsycoca 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 119336: Convert FrameSvg to 9 textures: different approach
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119336/#review62657 --- src/declarativeimports/core/framesvgitem.cpp https://git.reviewboard.kde.org/r/119336/#comment43435 Why a timer? Just call doUpdate.. - Mark Gaiser On jul 17, 2014, 8:17 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119336/ --- (Updated jul 17, 2014, 8:17 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a derivative of https://git.reviewboard.kde.org/r/119330/ It has a much simpler codebase and it doesn't touch framesvg in the library (and doesn't make things public) the important differences are two, that are imo 100% necessary to maintain a pixel perfect rendering (sacrificing that is a regression simply not acceptable in any case, even for non default themes, ever). At the same time avoids duplication of framesvg code in framesvgitem. potential issues: * e037203748 support for tiling still need porting * yes, it uses a qpainter, that means another copy: but again is necessary as framesvg knows how to render the end result pixel perfect, since for many themes the end piece is *not* the simple rendered element id. * yes, the final scaled texture is still uploaded as a whole, it only avoids to do it too often (like in animations) with event compression, again, only way to be sure it's correctly rendered all the time. The latter two points can have the following optimizations: Iff the frame does not have an overlay and does not have composeoverborders set, the following can be done: * use Svg::image() to fetch the pixmap of the piece, since in that case would be valid * resize the framesvg one single time, at a fixed size , like something 256x256 (256 is not random thing, since we have 8 bits per channel, usually gradients will have no more than 256 stops) and disable the resize timer, this way we are even sure that only one image per prefix will be stored in cache I should add regarding the last two optimization points: i would like to see some real benchmarks about them, or i would not consider then necessary until then. Diffs - tests/dialog.qml PRE-CREATION tests/testborders.qml PRE-CREATION examples/applets/widgetgallery/contents/ui/Buttons.qml 379585f examples/applets/widgetgallery/contents/ui/Menu.qml 1336c42 examples/applets/widgetgallery/contents/ui/standalonemain.qml PRE-CREATION src/declarativeimports/core/framesvgitem.h e155f6a src/declarativeimports/core/framesvgitem.cpp 8320212 src/declarativeimports/core/svgitem.cpp 1ed0631 Diff: https://git.reviewboard.kde.org/r/119336/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 119331: Use CMAKE_INSTALL_FULL_LIBEXECDIR_KF5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119331/ --- (Updated July 18, 2014, 2:51 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kio Description --- Resolves the problem of passing relative vs. absolute KF5_LIBEXEC_INSTALL_DIR/LIBEXEC_INSTALL_DIR. Diffs - autotests/CMakeLists.txt a9075e3 autotests/krununittest.cpp 9bbceb2 src/core/config-kiocore.h.cmake 6041c9d src/core/desktopexecparser.cpp be62791 src/core/slave.cpp c842f01 src/ioslaves/http/config-kioslave-http.h.cmake 3f313e9 src/ioslaves/http/http.cpp 55a19f4 src/kpac/config-kpac.h.cmake 1e73657 src/kpac/discovery.cpp d108fee Diff: https://git.reviewboard.kde.org/r/119331/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
