Re: KF 5.6 changelog
El Dimarts, 20 de gener de 2015, a les 13:49:14, Marco Martin va escriure: On Thursday 08 January 2015, Daniel Vrátil wrote: On Saturday, January 03, 2015 02:16:30 PM David Faure wrote: With cookies to Kai Uwe Broulik for using CHANGELOG: in most of his commits! The KPackage frameworks is Tier 2 and depends on KDocTools, which is also Tier 2, which, if I read Tier 2 policy correctly, is not allowed :-) Is there a plan to move it to Tier 3, or remove the dependency? Sorry if this was discussed before. if also optional is a problem, it can be just removed Or just leave it as tier 3? Cheers, Albert ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122194: Initialise all member variables
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122194/ --- Review request for KDE Frameworks. Repository: kdeclarative Description --- If DeclarativeMimeData is created using one of the two constructors m_source was not initialised. BUG: 336834 Diffs - src/qmlcontrols/draganddrop/DeclarativeMimeData.h 3d2beb5 src/qmlcontrols/draganddrop/DeclarativeMimeData.cpp fc96c83 Diff: https://git.reviewboard.kde.org/r/122194/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122194: Initialise all member variables
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122194/#review74508 --- Ship it! Ship It! - Milian Wolff On Jan. 21, 2015, 10:19 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122194/ --- (Updated Jan. 21, 2015, 10:19 p.m.) Review request for KDE Frameworks. Repository: kdeclarative Description --- If DeclarativeMimeData is created using one of the two constructors m_source was not initialised. BUG: 336834 Diffs - src/qmlcontrols/draganddrop/DeclarativeMimeData.h 3d2beb5 src/qmlcontrols/draganddrop/DeclarativeMimeData.cpp fc96c83 Diff: https://git.reviewboard.kde.org/r/122194/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122177: some minor fixes for windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122177/#review74509 --- Ship it! Ship It! - Allen Winter On Jan. 21, 2015, 10:39 p.m., Patrick Spendrin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122177/ --- (Updated Jan. 21, 2015, 10:39 p.m.) Review request for KDE Frameworks. Repository: kglobalaccel Description --- some minor fixes for windows Diffs - src/runtime/CMakeLists.txt a8fbfa1612a86f7371bf56926ec46fe14422a73a src/runtime/main.cpp f9cf9b353f00f1f68906d97f61cea5314a3663e4 Diff: https://git.reviewboard.kde.org/r/122177/diff/ Testing --- windows msvc 2013 Thanks, Patrick Spendrin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to normal : kded_master_qt5 #91
See http://build.kde.org/job/kded_master_qt5/91/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122177: some minor fixes for windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122177/ --- (Updated Jan. 21, 2015, 10:39 nachm.) Review request for KDE Frameworks. Changes --- use qunsetenv. Repository: kglobalaccel Description --- some minor fixes for windows Diffs (updated) - src/runtime/CMakeLists.txt a8fbfa1612a86f7371bf56926ec46fe14422a73a src/runtime/main.cpp f9cf9b353f00f1f68906d97f61cea5314a3663e4 Diff: https://git.reviewboard.kde.org/r/122177/diff/ Testing --- windows msvc 2013 Thanks, Patrick Spendrin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122177: some minor fixes for windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122177/ --- (Updated Jan. 21, 2015, 10:56 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kglobalaccel Description --- some minor fixes for windows Diffs - src/runtime/CMakeLists.txt a8fbfa1612a86f7371bf56926ec46fe14422a73a src/runtime/main.cpp f9cf9b353f00f1f68906d97f61cea5314a3663e4 Diff: https://git.reviewboard.kde.org/r/122177/diff/ Testing --- windows msvc 2013 Thanks, Patrick Spendrin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to normal : kded_stable_qt5 #12
See http://build.kde.org/job/kded_stable_qt5/12/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Build failed in Jenkins: kded_stable_qt5 #11
See http://build.kde.org/job/kded_stable_qt5/11/changes
Changes:
[shafff] qstring optimizations
--
Started by remote host 2a01:4f8:160:9363::9 with note: Triggered by commit
Building remotely on LinuxSlave - 1 (PACKAGER LINBUILDER) in workspace
http://build.kde.org/job/kded_stable_qt5/ws/
Running Prebuild steps
[kded_stable_qt5] $ /bin/sh -xe /tmp/hudson8710086524745518377.sh
+ /home/jenkins/scripts/setup-env.sh
Preparing to perform KDE Continuous Integration build
== Setting Up Sources
Cloning into '.'...
Branch jenkins set up to track remote branch master from origin.
== Cleaning Source Tree
HEAD is now at fa4df1d qstring optimizations
Success build forhudson.tasks.Shell@6e38dbdb
git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
git config remote.origin.url git://anongit.kde.org/kded # timeout=10
Fetching upstream changes from git://anongit.kde.org/kded
git --version # timeout=10
git -c core.askpass=true fetch --tags --progress git://anongit.kde.org/kded
+refs/heads/*:refs/remotes/origin/*
git rev-parse refs/remotes/origin/jenkins^{commit} # timeout=10
git rev-parse refs/remotes/origin/refs/heads/jenkins^{commit} # timeout=10
git rev-parse refs/heads/jenkins^{commit} # timeout=10
Checking out Revision fa4df1d8812464480a9ee40d13e48f0a39c5e0f8
(refs/heads/jenkins)
git config core.sparsecheckout # timeout=10
git checkout -f fa4df1d8812464480a9ee40d13e48f0a39c5e0f8
git rev-list ce782e39eb02f41944cfdf5d9d0d82fc5cdf9616 # timeout=10
git tag -a -f -m Jenkins Build #11 jenkins-kded_stable_qt5-11 # timeout=10
Run condition [File exists] enabling prebuild for step [Publish JUnit test
result report]
Run condition [File exists] enabling prebuild for step [Publish Cppcheck
results]
[kded_stable_qt5] $ /bin/sh -xe /tmp/hudson7555039205595089452.sh
+ /home/jenkins/scripts/execute-job.sh
KDE Continuous Integration Build
== Building Project: kded - Branch master
== Build Dependencies:
kwallet - Branch master
attica - Branch master
kconfig - Branch master
kio - Branch master
kcrash - Branch master
ktextwidgets - Branch master
kauth - Branch master
kcoreaddons - Branch master
kcompletion - Branch master
ki18n - Branch master
qt5 - Branch 5.3.2
sonnet - Branch master
solid - Branch master
kwidgetsaddons - Branch master
kconfigwidgets - Branch master
cmake - Branch master
kwindowsystem - Branch master
dogtail - Branch master
knotifications - Branch master
kinit - Branch master
phonon - Branch master
kdoctools - Branch master
extra-cmake-modules - Branch master
kguiaddons - Branch master
kjobwidgets - Branch master
kdbusaddons - Branch master
kxmlgui - Branch master
polkit-qt-1 - Branch master
kitemviews - Branch master
kbookmarks - Branch master
kservice - Branch master
kcodecs - Branch master
kglobalaccel - Branch master
kiconthemes - Branch master
karchive - Branch master
== Applying Patches
=== No patches to apply
== Syncing Dependencies from Master Server
== Configuring Build
-- The C compiler identification is GNU 4.8.2
-- The CXX compiler identification is GNU 4.8.2
-- Check for working C compiler: /home/jenkins/bin/cc
-- Check for working C compiler: /home/jenkins/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /home/jenkins/bin/c++
-- Check for working CXX compiler: /home/jenkins/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for __GLIBC__
-- Looking for __GLIBC__ - found
-- Performing Test _OFFT_IS_64BIT
-- Performing Test _OFFT_IS_64BIT - Success
-- Found Gettext: /usr/bin/msgmerge (found version 0.18.1)
-- Found PythonInterp: /usr/bin/python (found version 2.7.3)
--
-- The following REQUIRED packages have been found:
* ECM (required version = 1.6.0)
* Qt5Core (required version = 5.3.2)
* Qt5DBus
* Qt5Gui (required version = 5.3.2)
* Qt5Widgets
* Qt5 (required version = 5.2)
* KF5Config (required version = 5.6.0)
* KF5CoreAddons (required version = 5.6.0)
* KF5Crash (required version = 5.6.0)
* KF5DBusAddons (required version = 5.6.0)
* KF5DocTools (required version = 5.6.0)
* KF5Init (required version = 5.6.0)
* Gettext
* PythonInterp
* KF5Service (required version = 5.6.0)
-- Configuring done
-- Generating done
CMake Warning:
Manually-specified variables were not used by the project:
KDE4_BUILD_TESTS
LIB_SUFFIX
SIP_DEFAULT_SIP_DIR
-- Build files have been written to:
http://build.kde.org/job/kded_stable_qt5/ws/build
== Commencing the Build
Scanning dependencies of target docs-kded5-kded5-8
Scanning
Build failed in Jenkins: kded_master_qt5 #90
See http://build.kde.org/job/kded_master_qt5/90/changes
Changes:
[shafff] qstring optimizations
--
Started by remote host 2a01:4f8:160:9363::9 with note: Triggered by commit
Building remotely on LinuxSlave - 3 (PACKAGER LINBUILDER) in workspace
http://build.kde.org/job/kded_master_qt5/ws/
Running Prebuild steps
[kded_master_qt5] $ /bin/sh -xe /tmp/hudson6807147741392306095.sh
+ /home/jenkins/scripts/setup-env.sh
Preparing to perform KDE Continuous Integration build
== Setting Up Sources
Cloning into '.'...
Branch jenkins set up to track remote branch master from origin.
== Cleaning Source Tree
HEAD is now at fa4df1d qstring optimizations
Success build forhudson.tasks.Shell@4a1b3eb2
git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
git config remote.origin.url git://anongit.kde.org/kded # timeout=10
Fetching upstream changes from git://anongit.kde.org/kded
git --version # timeout=10
git -c core.askpass=true fetch --tags --progress git://anongit.kde.org/kded
+refs/heads/*:refs/remotes/origin/*
git rev-parse refs/remotes/origin/jenkins^{commit} # timeout=10
git rev-parse refs/remotes/origin/refs/heads/jenkins^{commit} # timeout=10
git rev-parse refs/heads/jenkins^{commit} # timeout=10
Checking out Revision fa4df1d8812464480a9ee40d13e48f0a39c5e0f8
(refs/heads/jenkins)
git config core.sparsecheckout # timeout=10
git checkout -f fa4df1d8812464480a9ee40d13e48f0a39c5e0f8
git rev-list ce782e39eb02f41944cfdf5d9d0d82fc5cdf9616 # timeout=10
git tag -a -f -m Jenkins Build #90 jenkins-kded_master_qt5-90 # timeout=10
Run condition [File exists] enabling prebuild for step [Publish JUnit test
result report]
Run condition [File exists] enabling prebuild for step [Publish Cppcheck
results]
[kded_master_qt5] $ /bin/sh -xe /tmp/hudson991731441625017283.sh
+ /home/jenkins/scripts/execute-job.sh
KDE Continuous Integration Build
== Building Project: kded - Branch master
== Build Dependencies:
kwallet - Branch master
attica - Branch master
kconfig - Branch master
kxmlgui - Branch master
kio - Branch master
kglobalaccel - Branch master
ktextwidgets - Branch master
kauth - Branch master
kguiaddons - Branch master
kcompletion - Branch master
phonon - Branch master
kdoctools - Branch master
sonnet - Branch master
kconfigwidgets - Branch master
kwidgetsaddons - Branch master
solid - Branch master
kitemviews - Branch master
kwindowsystem - Branch master
polkit-qt-1 - Branch master
knotifications - Branch master
cmake - Branch master
kinit - Branch master
kservice - Branch master
extra-cmake-modules - Branch master
kbookmarks - Branch master
kdbusaddons - Branch master
kcoreaddons - Branch master
dogtail - Branch master
qt5 - Branch 5.4.0
kjobwidgets - Branch master
kcodecs - Branch master
kcrash - Branch master
kiconthemes - Branch master
ki18n - Branch master
karchive - Branch master
== Applying Patches
=== No patches to apply
== Syncing Dependencies from Master Server
== Configuring Build
-- The C compiler identification is GNU 4.8.2
-- The CXX compiler identification is GNU 4.8.2
-- Check for working C compiler: /home/jenkins/bin/cc
-- Check for working C compiler: /home/jenkins/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /home/jenkins/bin/c++
-- Check for working CXX compiler: /home/jenkins/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for __GLIBC__
-- Looking for __GLIBC__ - found
-- Performing Test _OFFT_IS_64BIT
-- Performing Test _OFFT_IS_64BIT - Success
-- Found Gettext: /usr/bin/msgmerge (found version 0.18.1)
-- Found PythonInterp: /usr/bin/python (found version 2.7.3)
--
-- The following REQUIRED packages have been found:
* ECM (required version = 1.6.0)
* Qt5Core (required version = 5.3.2)
* Qt5DBus
* Qt5Gui (required version = 5.3.2)
* Qt5Widgets
* Qt5 (required version = 5.2)
* KF5Config (required version = 5.6.0)
* KF5CoreAddons (required version = 5.6.0)
* KF5Crash (required version = 5.6.0)
* KF5DBusAddons (required version = 5.6.0)
* KF5DocTools (required version = 5.6.0)
* KF5Init (required version = 5.6.0)
* Gettext
* PythonInterp
* KF5Service (required version = 5.6.0)
-- Configuring done
-- Generating done
CMake Warning:
Manually-specified variables were not used by the project:
KDE4_BUILD_TESTS
LIB_SUFFIX
SIP_DEFAULT_SIP_DIR
-- Build files have been written to:
http://build.kde.org/job/kded_master_qt5/ws/build
== Commencing the Build
Scanning dependencies of target kded5_automoc
Scanning
Re: Review Request 122194: Initialise all member variables
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122194/#review74512 --- Ship it! Ship It! - Aleix Pol Gonzalez On Jan. 21, 2015, 11:19 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122194/ --- (Updated Jan. 21, 2015, 11:19 p.m.) Review request for KDE Frameworks. Repository: kdeclarative Description --- If DeclarativeMimeData is created using one of the two constructors m_source was not initialised. BUG: 336834 Diffs - src/qmlcontrols/draganddrop/DeclarativeMimeData.h 3d2beb5 src/qmlcontrols/draganddrop/DeclarativeMimeData.cpp fc96c83 Diff: https://git.reviewboard.kde.org/r/122194/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build became unstable: kservice_stable_qt5 #26
See http://build.kde.org/job/kservice_stable_qt5/26/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
On Jan. 20, 2015, 4:16 p.m., David Faure wrote:
src/kcmoduleproxy.cpp, line 99
https://git.reviewboard.kde.org/r/122168/diff/1/?file=343470#file343470line99
... while this line is happy with dots.
Hugo Pereira Da Costa wrote:
ok. Got it now. Thanks !
Hugo Pereira Da Costa wrote:
Stupid question: does it not also apply to the two lines above
(replacement of '-' and '/' by '_') ? And if yes, should I also 'fix' that in
the same review ?
Hugo Pereira Da Costa wrote:
ok. Ditch this. While trying not to do the replacements for dbusservice
(code below), it seems that '.' are also not allowed here. The crash I get
is:
process 7571: arguments to dbus_message_new_method_call() were incorrect,
assertion destination == NULL || _dbus_check_is_valid_bus_name
(destination) failed in file dbus-message.c line 1266.
For the code:
QString name = modInfo.handle();
name.replace(QLatin1Char('-'), QLatin1Char('_')); // hyphen is
not allowed in dbus, only [A-Z][a-z][0-9]_
name.replace(QLatin1Char('/'), QLatin1Char('_')); // same goes
for '/'
dbusService = QLatin1String(.kde.internal.KSettingsWidget_) +
name;
// for the path, we also need to replace '.' characters, which
are not allowed in dbus
name.replace(QLatin1Char('.'), QLatin1Char('_'));
dbusPath = QLatin1String(/internal/KSettingsWidget/) + name;
David Faure wrote:
so what's the actual dbusService string that you're trying to register?
http://dbus.freedesktop.org/doc/dbus-specification.html section Bus
names has the list of rules.
The library path (in the .desktop file) is
X-KDE-Library=org.kde.kdecoration2/breezedecoration
(the ork.kde.kdecoration2 subpath is a requirement from kwin)
Right now (without patch) the current code converts this into
dbusPath = /internal/KSettingsWidget/org.kde.kdecoration2_breezedecoration -
crash
dbusservice =
org.kde.internal.KSettingsWidget_org.kde.kdecoration2_breezedecoration
which also crashes
- Hugo
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/#review74403
---
On Jan. 20, 2015, 11:02 a.m., Hugo Pereira Da Costa wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/
---
(Updated Jan. 20, 2015, 11:02 a.m.)
Review request for KDE Frameworks.
Repository: kcmutils
Description
---
Also replace '.' by '_' when registering dbus path names from X_KDE_Library
Rationale:
kwin requires that configuration components for kdecoration gets installed as
kcmodule in $KF5/lib/plugins/org.kde.kdecoration2
Trying to make the said module for breeze directly loadable from kcmshell5, I
hit
ASSERT failure in QDBusConnection::registerObject: Invalid object path given
Diffs
-
src/kcmoduleproxy.cpp 630217a
Diff: https://git.reviewboard.kde.org/r/122168/diff/
Testing
---
yes, with code not pushed yet in breeze decoration config.
Before: crash (due to the above)
After: no crash.
Thanks,
Hugo Pereira Da Costa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
On Jan. 20, 2015, 4:16 p.m., David Faure wrote:
src/kcmoduleproxy.cpp, line 99
https://git.reviewboard.kde.org/r/122168/diff/1/?file=343470#file343470line99
... while this line is happy with dots.
Hugo Pereira Da Costa wrote:
ok. Got it now. Thanks !
Hugo Pereira Da Costa wrote:
Stupid question: does it not also apply to the two lines above
(replacement of '-' and '/' by '_') ? And if yes, should I also 'fix' that in
the same review ?
Hugo Pereira Da Costa wrote:
ok. Ditch this. While trying not to do the replacements for dbusservice
(code below), it seems that '.' are also not allowed here. The crash I get
is:
process 7571: arguments to dbus_message_new_method_call() were incorrect,
assertion destination == NULL || _dbus_check_is_valid_bus_name
(destination) failed in file dbus-message.c line 1266.
For the code:
QString name = modInfo.handle();
name.replace(QLatin1Char('-'), QLatin1Char('_')); // hyphen is
not allowed in dbus, only [A-Z][a-z][0-9]_
name.replace(QLatin1Char('/'), QLatin1Char('_')); // same goes
for '/'
dbusService = QLatin1String(.kde.internal.KSettingsWidget_) +
name;
// for the path, we also need to replace '.' characters, which
are not allowed in dbus
name.replace(QLatin1Char('.'), QLatin1Char('_'));
dbusPath = QLatin1String(/internal/KSettingsWidget/) + name;
so what's the actual dbusService string that you're trying to register?
http://dbus.freedesktop.org/doc/dbus-specification.html section Bus names has
the list of rules.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/#review74403
---
On Jan. 20, 2015, 11:02 a.m., Hugo Pereira Da Costa wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/
---
(Updated Jan. 20, 2015, 11:02 a.m.)
Review request for KDE Frameworks.
Repository: kcmutils
Description
---
Also replace '.' by '_' when registering dbus path names from X_KDE_Library
Rationale:
kwin requires that configuration components for kdecoration gets installed as
kcmodule in $KF5/lib/plugins/org.kde.kdecoration2
Trying to make the said module for breeze directly loadable from kcmshell5, I
hit
ASSERT failure in QDBusConnection::registerObject: Invalid object path given
Diffs
-
src/kcmoduleproxy.cpp 630217a
Diff: https://git.reviewboard.kde.org/r/122168/diff/
Testing
---
yes, with code not pushed yet in breeze decoration config.
Before: crash (due to the above)
After: no crash.
Thanks,
Hugo Pereira Da Costa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
On Jan. 20, 2015, 4:16 p.m., David Faure wrote:
src/kcmoduleproxy.cpp, line 99
https://git.reviewboard.kde.org/r/122168/diff/1/?file=343470#file343470line99
... while this line is happy with dots.
Hugo Pereira Da Costa wrote:
ok. Got it now. Thanks !
Hugo Pereira Da Costa wrote:
Stupid question: does it not also apply to the two lines above
(replacement of '-' and '/' by '_') ? And if yes, should I also 'fix' that in
the same review ?
Hugo Pereira Da Costa wrote:
ok. Ditch this. While trying not to do the replacements for dbusservice
(code below), it seems that '.' are also not allowed here. The crash I get
is:
process 7571: arguments to dbus_message_new_method_call() were incorrect,
assertion destination == NULL || _dbus_check_is_valid_bus_name
(destination) failed in file dbus-message.c line 1266.
For the code:
QString name = modInfo.handle();
name.replace(QLatin1Char('-'), QLatin1Char('_')); // hyphen is
not allowed in dbus, only [A-Z][a-z][0-9]_
name.replace(QLatin1Char('/'), QLatin1Char('_')); // same goes
for '/'
dbusService = QLatin1String(.kde.internal.KSettingsWidget_) +
name;
// for the path, we also need to replace '.' characters, which
are not allowed in dbus
name.replace(QLatin1Char('.'), QLatin1Char('_'));
dbusPath = QLatin1String(/internal/KSettingsWidget/) + name;
David Faure wrote:
so what's the actual dbusService string that you're trying to register?
http://dbus.freedesktop.org/doc/dbus-specification.html section Bus
names has the list of rules.
Hugo Pereira Da Costa wrote:
The library path (in the .desktop file) is
X-KDE-Library=org.kde.kdecoration2/breezedecoration
(the ork.kde.kdecoration2 subpath is a requirement from kwin)
Right now (without patch) the current code converts this into
dbusPath =
/internal/KSettingsWidget/org.kde.kdecoration2_breezedecoration - crash
dbusservice =
org.kde.internal.KSettingsWidget_org.kde.kdecoration2_breezedecoration
which also crashes
ok. Sorry. it seems I cannot reproduce the 'second' crash above, after a system
reboot.
I'll submit an updated patch in a sec. (reading the doc, it is true that the
said dbusservice, including dots, should be valid.
- Hugo
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/#review74403
---
On Jan. 20, 2015, 11:02 a.m., Hugo Pereira Da Costa wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/
---
(Updated Jan. 20, 2015, 11:02 a.m.)
Review request for KDE Frameworks.
Repository: kcmutils
Description
---
Also replace '.' by '_' when registering dbus path names from X_KDE_Library
Rationale:
kwin requires that configuration components for kdecoration gets installed as
kcmodule in $KF5/lib/plugins/org.kde.kdecoration2
Trying to make the said module for breeze directly loadable from kcmshell5, I
hit
ASSERT failure in QDBusConnection::registerObject: Invalid object path given
Diffs
-
src/kcmoduleproxy.cpp 630217a
Diff: https://git.reviewboard.kde.org/r/122168/diff/
Testing
---
yes, with code not pushed yet in breeze decoration config.
Before: crash (due to the above)
After: no crash.
Thanks,
Hugo Pereira Da Costa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122168/ --- (Updated Jan. 21, 2015, 10:57 a.m.) Review request for KDE Frameworks. Changes --- only replace '.' in dbus path name, not in service name Repository: kcmutils Description --- Also replace '.' by '_' when registering dbus path names from X_KDE_Library Rationale: kwin requires that configuration components for kdecoration gets installed as kcmodule in $KF5/lib/plugins/org.kde.kdecoration2 Trying to make the said module for breeze directly loadable from kcmshell5, I hit ASSERT failure in QDBusConnection::registerObject: Invalid object path given Diffs (updated) - src/kcmoduleproxy.cpp 630217a Diff: https://git.reviewboard.kde.org/r/122168/diff/ Testing --- yes, with code not pushed yet in breeze decoration config. Before: crash (due to the above) After: no crash. Thanks, Hugo Pereira Da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122144: Add new overload to KWindowSystem::icon
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122144/ --- (Updated Jan. 21, 2015, 8:58 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kwindowsystem Description --- The new overload is a special solution for the X11 platform. It takes the NETWinInfo as argument to read the information from. This significantly reduces the time to fetch icons for users who already have a NETWinInfo with the required data. E.g. for the case of KWin it can reduce the number of roundtrips to the X Server from up to 15 to 0. For non-X11 platforms the method just delegates to the method not taking the NETWinInfo argument. CHANGELOG: New overload to KWindowSystem::icon which reduces roundtrips to X-Server Diffs - src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b Diff: https://git.reviewboard.kde.org/r/122144/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 122140: Support reading IconPixmap and IconMask from WM_HINTS in NETWinInfo
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122140/ --- (Updated Jan. 21, 2015, 8:58 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kwindowsystem Description --- WM_HINTS property is already read and this provides the icon pixmap and icon mask hint as read from WM_HINTS. This can be used for example in KWindowSystem::icon which currently uses an XLib call. Use NETWinInfo's WM2WindowClass and WM2IconPixmap in KWindowSystemPrivateX11::icon Instead of using an XLib call the wrapper from NETWinInfo is used. This reduces the number of roundtrips to the X server in case flags includes both NETWM, ClassHint and WMHints. In additon we don't need the deprecated x error eater any more. Diffs - autotests/netwininfotestclient.cpp a0fdc403bb8329ea9fde786494bc0ccf88a8ebfd src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/netwm.h 382ab460867905fdf0a4106a271e6614478fb438 src/netwm.cpp 1c96aaf9e480842c0b5379c89722f2e91a664476 src/netwm_def.h 49d02035eaec402a61e0b17b377b27c80d605b7b src/netwm_p.h 0f1dd62ecd9c856e028dd33d28d461ed7099f60b Diff: https://git.reviewboard.kde.org/r/122140/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 122086: Add new method KWindowSystem::icons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74460 --- ping - do we want this approach to go in? - Martin Gräßlin On Jan. 21, 2015, 10:06 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 21, 2015, 10:06 a.m.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/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 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
On Jan. 20, 2015, 4:16 p.m., David Faure wrote:
src/kcmoduleproxy.cpp, line 99
https://git.reviewboard.kde.org/r/122168/diff/1/?file=343470#file343470line99
... while this line is happy with dots.
Hugo Pereira Da Costa wrote:
ok. Got it now. Thanks !
Hugo Pereira Da Costa wrote:
Stupid question: does it not also apply to the two lines above
(replacement of '-' and '/' by '_') ? And if yes, should I also 'fix' that in
the same review ?
ok. Ditch this. While trying not to do the replacements for dbusservice (code
below), it seems that '.' are also not allowed here. The crash I get is:
process 7571: arguments to dbus_message_new_method_call() were incorrect,
assertion destination == NULL || _dbus_check_is_valid_bus_name (destination)
failed in file dbus-message.c line 1266.
For the code:
QString name = modInfo.handle();
name.replace(QLatin1Char('-'), QLatin1Char('_')); // hyphen is not
allowed in dbus, only [A-Z][a-z][0-9]_
name.replace(QLatin1Char('/'), QLatin1Char('_')); // same goes for '/'
dbusService = QLatin1String(.kde.internal.KSettingsWidget_) + name;
// for the path, we also need to replace '.' characters, which are not
allowed in dbus
name.replace(QLatin1Char('.'), QLatin1Char('_'));
dbusPath = QLatin1String(/internal/KSettingsWidget/) + name;
- Hugo
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/#review74403
---
On Jan. 20, 2015, 11:02 a.m., Hugo Pereira Da Costa wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/
---
(Updated Jan. 20, 2015, 11:02 a.m.)
Review request for KDE Frameworks.
Repository: kcmutils
Description
---
Also replace '.' by '_' when registering dbus path names from X_KDE_Library
Rationale:
kwin requires that configuration components for kdecoration gets installed as
kcmodule in $KF5/lib/plugins/org.kde.kdecoration2
Trying to make the said module for breeze directly loadable from kcmshell5, I
hit
ASSERT failure in QDBusConnection::registerObject: Invalid object path given
Diffs
-
src/kcmoduleproxy.cpp 630217a
Diff: https://git.reviewboard.kde.org/r/122168/diff/
Testing
---
yes, with code not pushed yet in breeze decoration config.
Before: crash (due to the above)
After: no crash.
Thanks,
Hugo Pereira Da Costa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 21, 2015, 10:06 a.m.) Review request for KDE Frameworks and kwin. Changes --- added kwin again Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/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
Review Request 122179: kded: port to KDEDModule::moduleForMessage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122179/ --- Review request for KDE Frameworks and Kevin Ottens. Repository: kded Description --- kded: port to KDEDModule::moduleForMessage Diffs - src/kded.cpp 40e549f6443d5cd2b26c5f9bad9695462a3467aa Diff: https://git.reviewboard.kde.org/r/122179/diff/ Testing --- module autoload still works 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 122178: kdbusaddons: add KDEDModule::moduleForMessage for code sharing between kded and kiod.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122178/#review74461 --- Another approach would be a base class for such daemons, with a virtual method for the module loading (since it's implemented a bit differently, to avoid sycoca in kiod). But the dbus messageFilter can only be a static function, which needs to call self()-loadModule(), so the singleton (pointer to that base class) would have to be in the library too I generally don't like such forcing a design on you library code. - David Faure On Jan. 21, 2015, 9:03 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122178/ --- (Updated Jan. 21, 2015, 9:03 a.m.) Review request for KDE Frameworks and Kevin Ottens. Repository: kdbusaddons Description --- This is what the DBus message filter uses in order to find out which module is being called, in order to auto-load it. Diffs - src/kdedmodule.h 706529a1639b17118abfd80a02b310dd2acbc00d src/kdedmodule.cpp 54611c52775803ac734b52a53d79d2c762315112 Diff: https://git.reviewboard.kde.org/r/122178/diff/ Testing --- autoloading modules in kiod and kded works Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to stable : kwindowsystem_stable_qt5 » All,LINBUILDER #17
See http://build.kde.org/job/kwindowsystem_stable_qt5/Variation=All,label=LINBUILDER/17/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
On Jan. 20, 2015, 4:16 p.m., David Faure wrote: src/kcmoduleproxy.cpp, line 99 https://git.reviewboard.kde.org/r/122168/diff/1/?file=343470#file343470line99 ... while this line is happy with dots. Hugo Pereira Da Costa wrote: ok. Got it now. Thanks ! Stupid question: does it not also apply to the two lines above (replacement of '-' and '/' by '_') ? And if yes, should I also 'fix' that in the same review ? - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122168/#review74403 --- On Jan. 20, 2015, 11:02 a.m., Hugo Pereira Da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122168/ --- (Updated Jan. 20, 2015, 11:02 a.m.) Review request for KDE Frameworks. Repository: kcmutils Description --- Also replace '.' by '_' when registering dbus path names from X_KDE_Library Rationale: kwin requires that configuration components for kdecoration gets installed as kcmodule in $KF5/lib/plugins/org.kde.kdecoration2 Trying to make the said module for breeze directly loadable from kcmshell5, I hit ASSERT failure in QDBusConnection::registerObject: Invalid object path given Diffs - src/kcmoduleproxy.cpp 630217a Diff: https://git.reviewboard.kde.org/r/122168/diff/ Testing --- yes, with code not pushed yet in breeze decoration config. Before: crash (due to the above) After: no crash. Thanks, Hugo Pereira Da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122178: kdbusaddons: add KDEDModule::moduleForMessage for code sharing between kded and kiod.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122178/ --- Review request for KDE Frameworks and Kevin Ottens. Repository: kdbusaddons Description --- This is what the DBus message filter uses in order to find out which module is being called, in order to auto-load it. Diffs - src/kdedmodule.h 706529a1639b17118abfd80a02b310dd2acbc00d src/kdedmodule.cpp 54611c52775803ac734b52a53d79d2c762315112 Diff: https://git.reviewboard.kde.org/r/122178/diff/ Testing --- autoloading modules in kiod and kded works 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 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122168/ --- (Updated Jan. 21, 2015, 12:38 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcmutils Description --- Also replace '.' by '_' when registering dbus path names from X_KDE_Library Rationale: kwin requires that configuration components for kdecoration gets installed as kcmodule in $KF5/lib/plugins/org.kde.kdecoration2 Trying to make the said module for breeze directly loadable from kcmshell5, I hit ASSERT failure in QDBusConnection::registerObject: Invalid object path given Diffs - src/kcmoduleproxy.cpp 630217a Diff: https://git.reviewboard.kde.org/r/122168/diff/ Testing --- yes, with code not pushed yet in breeze decoration config. Before: crash (due to the above) After: no crash. Thanks, Hugo Pereira Da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122168/#review74471 --- Ship it! Ship It! - David Faure On Jan. 21, 2015, 10:57 a.m., Hugo Pereira Da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122168/ --- (Updated Jan. 21, 2015, 10:57 a.m.) Review request for KDE Frameworks. Repository: kcmutils Description --- Also replace '.' by '_' when registering dbus path names from X_KDE_Library Rationale: kwin requires that configuration components for kdecoration gets installed as kcmodule in $KF5/lib/plugins/org.kde.kdecoration2 Trying to make the said module for breeze directly loadable from kcmshell5, I hit ASSERT failure in QDBusConnection::registerObject: Invalid object path given Diffs - src/kcmoduleproxy.cpp 630217a Diff: https://git.reviewboard.kde.org/r/122168/diff/ Testing --- yes, with code not pushed yet in breeze decoration config. Before: crash (due to the above) After: no crash. Thanks, Hugo Pereira Da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122180: [KUnitConversion] Unit::setUnitMultiplier: Do not call oneself
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122180/#review74480 --- Ship it! Ship It! - Aleix Pol Gonzalez On Jan. 21, 2015, 3:47 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122180/ --- (Updated Jan. 21, 2015, 3:47 p.m.) Review request for KDE Frameworks. Repository: kunitconversion Description --- KUnitConversion: Unit::setUnitMultiplier: Do not call oneself Diffs - src/unit.cpp 013b5d7 Diff: https://git.reviewboard.kde.org/r/122180/diff/ Testing --- Created a test case in another patch. We no longer crash. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
On Jan. 21, 2015, 9:07 vorm., Martin Gräßlin wrote: ping - do we want this approach to go in? I've two concerns about this. 1. the name icons implies several (it's unfortunate that ::icon isn't ::iconPixmap) - maybe windowIcon() or similar 2. the behavior how an icon is build up from multiple sources (isn't) right now, you could end up w/ one 16x16 NETWM icon pixmap. Considerations: NETWM and WM_HINTS could be more specific than an icon from the appname, the appname could though be more size complete and finally WM_HINTS would likely be ugly compared to the other two (bitmasked). Overmore it might be undesirable to end up w/ an icon that has completely different looks for different sizes. As we don't have a specific hint on the required target size, it gets harder to ever take an informed decision on what to return here, but one might want to at least check the sizes provided by NEWTM before returning early (if other sources are allowed by flags) - yes, I know that this is not a straight suggestion for improvement :-( - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74460 --- On Jan. 21, 2015, 9:06 vorm., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 21, 2015, 9:06 vorm.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/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
Review Request 122183: [KUnitConversion] Currency: Fetch the currency file properly
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/ --- Review request for KDE Frameworks. Bugs: 340819 https://bugs.kde.org/show_bug.cgi?id=340819 Repository: kunitconversion Description --- Currency: Fetch the currency file properly Properly run an event loop and wait for the file to be fetched. Also add a test to make sure currency conversion is working. This patch also contains - * https://git.reviewboard.kde.org/r/122182/ * https://git.reviewboard.kde.org/r/122181/ * https://git.reviewboard.kde.org/r/122180/ This is because reviewboard refuses to upload only a part of the diff. Please only look at currency.cpp w.r.t the EventLoop. Diffs - autotests/convertertest.h 8129a48 autotests/convertertest.cpp ae4298e src/currency.cpp 715233c src/unit.cpp 013b5d7 src/unitcategory.cpp c34217e Diff: https://git.reviewboard.kde.org/r/122183/diff/ Testing --- Test now passes. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122180: [KUnitConversion] Unit::setUnitMultiplier: Do not call oneself
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122180/ --- Review request for KDE Frameworks. Repository: kunitconversion Description --- KUnitConversion: Unit::setUnitMultiplier: Do not call oneself Diffs - src/unit.cpp 013b5d7 Diff: https://git.reviewboard.kde.org/r/122180/diff/ Testing --- Created a test case in another patch. We no longer crash. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122181: [KUnitConversion] UnitCategory convert: Call UnitCategoryPrivate instead of duplicating it
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122181/ --- Review request for KDE Frameworks. Repository: kunitconversion Description --- This is probably a mistake when implementing the private class. Both UnitCategoryPrivate::convert and UnitCategory::convert essentially the same thing. However, all sub categories modify the UnitCategoryPrivate virtual method, so we should be calling that one. This was caught while trying to debug the currency converter. It has its own custom convert function. Diffs - src/unitcategory.cpp c34217e Diff: https://git.reviewboard.kde.org/r/122181/diff/ Testing --- Fixes a test in another patch. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122182: [KUnitConversion] Currency: Give default values for various currencies
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122182/ --- Review request for KDE Frameworks. Repository: kunitconversion Description --- The currency converter works by fetching a file from the network every day and using those conversion values. If network is not available it will try and use the previous version of that file. If no file then it will use the default values. The default values was 1e+99 in all cases. This patch updates it to the current values as reflected in the file. Diffs - src/currency.cpp 715233c Diff: https://git.reviewboard.kde.org/r/122182/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build became unstable: plasma-framework_master_qt5 » All,LINBUILDER #965
See http://build.kde.org/job/plasma-framework_master_qt5/Variation=All,label=LINBUILDER/965/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
On Jan. 21, 2015, 10:07 a.m., Martin Gräßlin wrote: ping - do we want this approach to go in? Thomas Lübking wrote: I've two concerns about this. 1. the name icons implies several (it's unfortunate that ::icon isn't ::iconPixmap) - maybe windowIcon() or similar 2. the behavior how an icon is build up from multiple sources (isn't) right now, you could end up w/ one 16x16 NETWM icon pixmap. Considerations: NETWM and WM_HINTS could be more specific than an icon from the appname, the appname could though be more size complete and finally WM_HINTS would likely be ugly compared to the other two (bitmasked). Overmore it might be undesirable to end up w/ an icon that has completely different looks for different sizes. As we don't have a specific hint on the required target size, it gets harder to ever take an informed decision on what to return here, but one might want to at least check the sizes provided by NEWTM before returning early (if other sources are allowed by flags) - yes, I know that this is not a straight suggestion for improvement :-( I completely agree with your two concerns and also thought about the same. Concerning point 1 I wanted to make them another ::icon overload but that's not possible - it would be ambiguous :-(. windowIcon might indeed be a better name. Concerning point 2: I thought about combining all items. E.g. if NETWM and WM_HINTS are set, it will include both. But if NETWM provides a size which WM_HINTS doesn't it wouldn't be set. But that wouldn't work with the app or the X icon. I have to think a little bit more about it. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74460 --- On Jan. 21, 2015, 10:06 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 21, 2015, 10:06 a.m.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/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
