Re: Review Request: KConfigXT performance fix: no need to reparse when nothing to save
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103166/#review8410 --- Ship it! Ship It! - Laurent Montel On Nov. 17, 2011, 3:41 p.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103166/ --- (Updated Nov. 17, 2011, 3:41 p.m.) Review request for kdelibs and Aaron J. Seigo. Description --- Closing a kmail composer window takes a very long time, because it's calling writeConfig() on 7 kconfigskeleton objects (see KMKernel::slotSyncConfig), and each of them calls KConfig::sync() [that's ok, it doesn't do anything if there's no change to save, i.e. no dirty entry] followed by readConfig(), which calls KConfig::reparseConfiguration(). That's a lot of config file parsing, for a case where *nothing has changed*. Looking into the history of the kconfigskeleton code didn't help finding out why writeConfig calls readConfig, this was added by Waldo in the initial code in 2003. A real sync() with dirty entries triggers a reparsing (to merge changes on-disk with changes in memory), so it makes sense to propagate the changes from KConfig to the KConfigSkeleton member variables after a sync. But not when sync does nothing at all. Diffs - kdecore/config/kconfig.h 51381ca kdecore/config/kconfig.cpp fcf0ce9 kdecore/config/kcoreconfigskeleton.cpp f5dd4bf Diff: http://git.reviewboard.kde.org/r/103166/diff/diff Testing --- kdecore unittests pass. The testing of closing a composer window isn't fully fixed with this patch alone, kmail2rc is still reparsed because the kdeui code that calls revertToDefault on entries that are equal to the default value, makes the config dirty. This requires a different fix which I'll submit separately. Thanks, David Faure
Review Request: Fix library creation
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103212/ --- Review request for KDE Base Apps and Aaron J. Seigo. Description --- this patch create the library: libpowerdevilui.so.4* This allow to package kde-workspace. Diffs - powerdevil/daemon/CMakeLists.txt b3b6263 Diff: http://git.reviewboard.kde.org/r/103212/diff/diff Testing --- create a rpm without the patch : kdebase4-workspace-devel-4.7.80-3-mdv2012.0.i586 (due to unsatisfied devel(libpowerdevilui)) and it works with the modification. Thanks, Nicolas Lécureuil
Re: adding a runtime dependency into KDELIBS
El Dimecres, 23 de novembre de 2011, a les 00:00:29, Alex Fiestas va escriure: We're in the process of merging a review which will partly fix the sad situation of MTP/MPI/iPod devices in libsolid, the review I'm talking about is: https://git.reviewboard.kde.org/r/103028/ This as far as I know was (is) working with the linux-deprecated HAL backend, so is something we urge to fix. This patch adds support for media-player-info which is basically what replaces some features HAL has in the new u* stuff. So, Can I give the ship it for this patch? can we add an optional runtime dependency to kdelibs 4.7.x ? I don't see this adding any kind of dependency from the POV of code, that is, it does not do qprocess nor the likes, so can you explain what the optional runtime dependency means here? Also Kevin's code looks like copied/duplicated aka bad stuff. Albert Thanks.
Re: adding a runtime dependency into KDELIBS
2011/11/24 Albert Astals Cid aa...@kde.org: El Dimecres, 23 de novembre de 2011, a les 00:00:29, Alex Fiestas va escriure: We're in the process of merging a review which will partly fix the sad situation of MTP/MPI/iPod devices in libsolid, the review I'm talking about is: https://git.reviewboard.kde.org/r/103028/ This as far as I know was (is) working with the linux-deprecated HAL backend, so is something we urge to fix. This patch adds support for media-player-info which is basically what replaces some features HAL has in the new u* stuff. So, Can I give the ship it for this patch? can we add an optional runtime dependency to kdelibs 4.7.x ? I don't see this adding any kind of dependency from the POV of code, that is, it does not do qprocess nor the likes, so can you explain what the optional runtime dependency means here? I think the code is accessing the DBus service, even though in the patch this is not mentioned as well, so I'm slightly confused. Besides, on a different topic, we should really standardize for handling this kind of runtime dependencies, as I think they'll just grow more as time goes Also Kevin's code looks like copied/duplicated aka bad stuff. Albert Thanks.
Re: adding a runtime dependency into KDELIBS
2011/11/24 Dario Freddi drf54...@gmail.com: 2011/11/24 Albert Astals Cid aa...@kde.org: El Dimecres, 23 de novembre de 2011, a les 00:00:29, Alex Fiestas va escriure: We're in the process of merging a review which will partly fix the sad situation of MTP/MPI/iPod devices in libsolid, the review I'm talking about is: https://git.reviewboard.kde.org/r/103028/ This as far as I know was (is) working with the linux-deprecated HAL backend, so is something we urge to fix. This patch adds support for media-player-info which is basically what replaces some features HAL has in the new u* stuff. So, Can I give the ship it for this patch? can we add an optional runtime dependency to kdelibs 4.7.x ? I don't see this adding any kind of dependency from the POV of code, that is, it does not do qprocess nor the likes, so can you explain what the optional runtime dependency means here? I think the code is accessing the DBus service, Forget this bit, I was just confusing myself, sorry. From the wiki page: media-player-info is a repository of data files describing media player capabilities, mostly of mass-storage devices. These files contain information about the directory layout to use to add music to these devices, the supported file formats and so on. These capabilities used to be provided by HAL in the 10-usb-music-players.fdi file, but HAL is now deprecated, so the information is being provided as a separate package. So it's likely the new properties will be provided reliably only if that package is installed, which explains pretty much everything else. Also Kevin's code looks like copied/duplicated aka bad stuff. Albert Thanks.
Review Request: Proper password caching when opening remote directories in KFileDialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103226/ --- Review request for kdelibs and David Faure. Description --- The attached patch fixes the scenario outlined in bug# 179663 by making the best effort to identify and use the top most window when invoking KIO functions. That way any password information supplied by the user is cached, even if the user did not check the Remember password checkbox, for the duration of the application instead of just the lifetime of the file dialog. Right now almost all KFileDialog's KIO access does not set the widget parameter. If a site then requires authentication, no window-id information will be passed to it. That in turn results in the user supplied password being cached for only a very very short duration, ~10 secs. Afterwards, the password is removed and the user is inevitably re-prompted to supply the same credentials again. This addresses bug 179663. http://bugs.kde.org/show_bug.cgi?id=179663 Diffs - kfile/kdiroperator.cpp 4c93ac9 kfile/kdirselectdialog.cpp 0212e58 kfile/kfilewidget.cpp 09b86d4 kfile/knewfilemenu.cpp ac54041 Diff: http://git.reviewboard.kde.org/r/103226/diff/diff Testing --- Tested with the scenario outlined in the afforementioned bug report using a publicly available demo webdav server, webdav://demo.sabredav.org. Thanks, Dawit Alemayehu
Re: Review Request: Proper password caching when opening remote directories in KFileDialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103226/ --- (Updated Nov. 24, 2011, 7:37 a.m.) Review request for kdelibs and David Faure. Description --- The attached patch fixes the scenario outlined in bug# 179663 by making the best effort to identify and use the top most window when invoking KIO functions. That way any password information supplied by the user is cached, even if the user did not check the Remember password checkbox, for the duration of the application instead of just the lifetime of the file dialog. Right now almost all KFileDialog's KIO access does not set the widget parameter. If a site then requires authentication, no window-id information will be passed to it. That in turn results in the user supplied password being cached for only a very very short duration, ~10 secs. Afterwards, the password is removed and the user is inevitably re-prompted to supply the same credentials again. This addresses bug 179663. http://bugs.kde.org/show_bug.cgi?id=179663 Diffs (updated) - kfile/kdiroperator.cpp 4c93ac9 kfile/kdirselectdialog.cpp 0212e58 kfile/kfilewidget.cpp 09b86d4 kfile/knewfilemenu.cpp ac54041 Diff: http://git.reviewboard.kde.org/r/103226/diff/diff Testing --- Tested with the scenario outlined in the afforementioned bug report using a publicly available demo webdav server, webdav://demo.sabredav.org. Thanks, Dawit Alemayehu
