Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 2:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 2:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122198: Add support for _NET_WM_OPAQUE_REGION
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122198/#review74617 --- Ship it! I assume there's requirement for a setter as well (rather than even a getter)? - Thomas Lübking On Jan. 22, 2015, 1:49 nachm., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122198/ --- (Updated Jan. 22, 2015, 1:49 nachm.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- NETWinInfo provides the opaque region as a vector of NETRect. CHANGELOG: support for _NET_WM_OPAQUE_REGION Diffs - src/netwm.cpp 600b0f6edbd42dd57f9bc2c1ce5dbc91844d344d src/netwm_def.h 3066726699947b3415433107e88a049584f3ebbc src/netwm_p.h e709caf16481938bb9f93083139ec3ea49e0bcb7 autotests/netrootinfotestwm.cpp c88ba0a10555181b8b80c9156e587185455d5047 autotests/netwininfotestclient.cpp cebf93b50a8917d243523269ecfe117f343d7f30 src/netwm.h 7729294286b3ec6dc94684ffde36b32eace7ae0d Diff: https://git.reviewboard.kde.org/r/122198/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 122206: [kio] Make tests optional
On gen. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. Matthew Dawson wrote: I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. Your logic is flawed, you say building autotests without finding Qt5Test is only going to break builds. That is correct, just that Qt5Test is being searched for, so your rationale for applying the patch is moot. - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On gen. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote:
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. - Andreas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. Matthew Dawson wrote: I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. Albert Astals Cid wrote: Your logic is flawed, you say building autotests without finding Qt5Test is only going to break builds. That is correct, just that Qt5Test is being searched for, so your rationale for applying the patch is moot. Sorry, I misread the patch. You are correct the build is fine as is. We should discuss this on the mailing list first before applying. - Matthew --- This is an automatically generated e-mail. To reply, visit:
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 2:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 2:48 p.m.) Review request for KDE
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 2:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 2:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ 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_stable_qt5 » All,LINBUILDER #76
See http://build.kde.org/job/plasma-framework_stable_qt5/Variation=All,label=LINBUILDER/76/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Change in kio[master]: KIO: provide own daemon (kiod) to run multiple services (mod...
David Faure has uploaded a new change for review.
https://gerrit.vesnicky.cesnet.cz/r/343
Change subject: KIO: provide own daemon (kiod) to run multiple services
(modules).
..
KIO: provide own daemon (kiod) to run multiple services (modules).
Based on the kded idea, but without the KSycoca dependency,
the kdeinit dependency, the dir watching for kbuildsycoca,
the workspace startup phases etc. Makes KIO more independent.
The modules still inherit KDEDModule, they are just hosted by another
daemon.
In kiod, the modules are only loaded on demand via DBus calls,
and must use json.
Followup commits will convert kcookiejar, kpasswdserver, proxyscout,
and favicons.
Change-Id: I85417d324125c4ba7e80761b9c0bd0e9296a1b6c
---
M src/CMakeLists.txt
M src/core/ksslcertificatemanager.cpp
A src/kiod/CMakeLists.txt
A src/kiod/kiod_main.cpp
A src/kiod/org.kde.kiod5.service.in
M src/kssld/CMakeLists.txt
M src/kssld/kssld.cpp
M src/kssld/kssld.desktop
8 files changed, 153 insertions(+), 22 deletions(-)
git pull ssh://gerrit.vesnicky.cesnet.cz:29418/kio refs/changes/43/343/1
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 6405e5c..a1d644d 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -1,8 +1,9 @@
add_subdirectory(core)
-# KIOCore-only libs
+# KIOCore-only executables
add_subdirectory(kntlm)
add_subdirectory(ioslaves)
+add_subdirectory(kiod)
add_subdirectory(kssld)
add_subdirectory(kioslave)
diff --git a/src/core/ksslcertificatemanager.cpp
b/src/core/ksslcertificatemanager.cpp
index 76198eb..7dca155 100644
--- a/src/core/ksslcertificatemanager.cpp
+++ b/src/core/ksslcertificatemanager.cpp
@@ -196,7 +196,7 @@
KSslCertificateManagerPrivate::KSslCertificateManagerPrivate()
: config(QString::fromLatin1(ksslcertificatemanager),
KConfig::SimpleConfig),
- iface(new org::kde::KSSLDInterface(QString::fromLatin1(org.kde.kded5),
+ iface(new org::kde::KSSLDInterface(QString::fromLatin1(org.kde.kssld5),
QString::fromLatin1(/modules/kssld),
QDBusConnection::sessionBus())),
isCertListLoaded(false),
diff --git a/src/kiod/CMakeLists.txt b/src/kiod/CMakeLists.txt
new file mode 100644
index 000..1ef3a25
--- /dev/null
+++ b/src/kiod/CMakeLists.txt
@@ -0,0 +1,19 @@
+set(kiod_SRCS kiod_main.cpp)
+
+add_executable(kiod5 ${kiod_SRCS})
+
+target_link_libraries(kiod5
+ KF5::KIOCore # ksslcertificatemanager
+ KF5::DBusAddons # kdedmodule
+ KF5::CoreAddons # kpluginfactory
+ Qt5::Network
+ Qt5::DBus
+)
+
+install(TARGETS kiod5 DESTINATION ${KF5_LIBEXEC_INSTALL_DIR})
+
+configure_file(org.kde.kiod5.service.in
+ ${CMAKE_CURRENT_BINARY_DIR}/org.kde.kiod5.service)
+install(FILES ${CMAKE_CURRENT_BINARY_DIR}/org.kde.kiod5.service
+ DESTINATION ${DBUS_SERVICES_INSTALL_DIR})
+
diff --git a/src/kiod/kiod_main.cpp b/src/kiod/kiod_main.cpp
new file mode 100644
index 000..474b04e
--- /dev/null
+++ b/src/kiod/kiod_main.cpp
@@ -0,0 +1,118 @@
+/* This file is part of the KDE libraries
+Copyright (C) 2015 David Faure fa...@kde.org
+
+This library is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2 of the License or (at
+your option) version 3 or, at the discretion of KDE e.V. (which shall
+act as a proxy as in section 14 of the GPLv3), any later version.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Library General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License
+along with this library; see the file COPYING.LIB. If not, write to
+the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+Boston, MA 02110-1301, USA.
+*/
+
+#include QCoreApplication
+#include QHash
+#include QDBusMessage
+#include QDBusConnectionInterface
+
+#include KDBusService
+#include KDEDModule
+#include KPluginLoader
+#include KPluginFactory
+#include KPluginMetaData
+
+#include QLoggingCategory
+Q_DECLARE_LOGGING_CATEGORY(KIOD_CATEGORY);
+Q_LOGGING_CATEGORY(KIOD_CATEGORY, kf5.kiod);
+
+class KIOD
+{
+public:
+KDEDModule *loadModule(const QString name);
+
+private:
+QHashQString, KDEDModule * m_modules;
+};
+
+KDEDModule *KIOD::loadModule(const QString name)
+{
+// Make sure this method is only called with valid module names.
+Q_ASSERT(name.indexOf('/') == -1);
+
+KDEDModule *module = m_modules.value(name, 0);
+if (module) {
+return module;
+}
+
+KPluginLoader loader(kf5/kiod/ + name);
+KPluginFactory *factory = loader.factory();
+if (factory) {
+module = factory-createKDEDModule();
+}
Jenkins build is back to stable : plasma-framework_master_qt5 » All,LINBUILDER #966
See http://build.kde.org/job/plasma-framework_master_qt5/Variation=All,label=LINBUILDER/966/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122204: Mark results as required.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/#review74632 --- Ship it! Ship It! - Chusslove Illich On Јан. 22, 2015, 7:34 по п., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/ --- (Updated Јан. 22, 2015, 7:34 по п.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This prevents API misuage, similar to how QString::arg is doing it. See also: http://quickgit.kde.org/?p=kdevplatform.gita=commith=078f1cd584e2de75ad19c46282b47dd1e0feff8f Diffs - src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 Diff: https://git.reviewboard.kde.org/r/122204/diff/ Testing --- Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122220: Don't show two progress dialogs for PasteJob
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/ --- (Updated Jan. 23, 2015, 10:34 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Bugs: 342276 https://bugs.kde.org/show_bug.cgi?id=342276 Repository: kio Description --- PasteJob creates a copy/move job and passes its flags to it. These jobs then register with the progress dialog, but so does the original PasteJob and then there are two progress bars/dialogs. This moves the registration to the job tracker only to the pasteMimeDataImpl job which does not take the flags of the parent job. I'm not sure if it makes sense there though. Alternatively it could be just removed. Diffs - src/widgets/pastejob_p.h f5e68d3 Diff: https://git.reviewboard.kde.org/r/10/diff/ Testing --- Copying and moving files no longer spawns two progress dialogs. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 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/#review74634 --- src/currency.cpp https://git.reviewboard.kde.org/r/122183/#comment51724 can be removed now src/currency.cpp https://git.reviewboard.kde.org/r/122183/#comment51725 (btw KIO is threadsafe now, this should work) src/currency.cpp https://git.reviewboard.kde.org/r/122183/#comment51727 pass 'this' as 3rd argument, just in case 'this' gets deleted while the job is running? src/currency.cpp https://git.reviewboard.kde.org/r/122183/#comment51726 I think reply-deleteLater() is missing too. - David Faure On Jan. 23, 2015, 2:17 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/ --- (Updated Jan. 23, 2015, 2:17 p.m.) 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 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
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74633 --- Ship it! Not sure why this is suddenly triggering many philosophical discussions about what users should be doing (IMHO, give them choice, this is opensource). Similar changes have been done in most other frameworks long ago, this is most certainly consistent. (see e.g. kcoreaddons - the find_package(Qt5Test) is being done in the autotests subfolder, the same could be done here). - David Faure On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122232: KConfig: fix using KSharedConfig in global object destructor.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122232/ --- Review request for KDE Frameworks, Albert Astals Cid and Matthew Dawson. Repository: kconfig Description --- kconfig_in_global_object.cpp comes from kdelibs4support (after porting to Q_GLOBAL_STATIC) ksharedconfig_in_global_object.cpp is new (but works with kdelibs4) and reproduces Albert's KgDifficulty testcase. Diffs - autotests/CMakeLists.txt b91f754b705fc87bb8b729bea72fbb5f7d427ace autotests/kconfig_in_global_object.cpp PRE-CREATION autotests/ksharedconfig_in_global_object.cpp PRE-CREATION src/core/kconfig.cpp 782e9714521234a3e3d8f3a788967e5c9a40f38a Diff: https://git.reviewboard.kde.org/r/122232/diff/ Testing --- Both tests pass and the QCoreApplication::arguments warning (because called after qApp is destroyed) is gone. 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 122105: Fix KgDifficulty saving on app close
On Monday 19 January 2015 21:43:41 Albert Astals Cid wrote: See http://quickgit.kde.org/?p=libkdegames.gita=blobh=b94cce00ef338cdfd3224e35 42074b699ead0b29hb=63855d630c928e46554885f6d022d0091dbe252bf=kgdifficulty. cpp It has a K_GLOBAL_STATIC(KgDifficulty, g_difficulty) and a KConfigGroup cg(KGlobal::config(), KgDifficulty); cg.writeEntry(Level, currentLevel()-key()); in KgDifficulty destructor. And it did work :D OK. So I investigated it further and well, it worked through luck, mostly :) As you probably heard before, the order of destruction of global statics is not something one should rely upon. But that's exactly what happened here. If the global static is created before KGlobal then it's destroyed afterwards, and asserts. If it's created after KGlobal then no problem. Since KGlobal was created in many automatic ways in kdelibs4, all apps ended up in the second case, which is why it worked. = working kdelibs4 testcase http://www.davidfaure.fr/2015/ksharedconfig_in_global_object.cpp In KF5 it gives the QCoreApplication::arguments issue, but works otherwise. Therefore one possible fix is ... calling KSharedConfig::openConfig() in main(), or at least before calling the code that ends up creating your singleton. Tricky (not really intuitive). Here's the fix for the QCoreApplication::arguments issue: https://git.reviewboard.kde.org/r/122232/ -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 2:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. Matthew Dawson wrote: I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. Albert Astals Cid wrote: Your logic is flawed, you say building autotests without finding Qt5Test is only going to break builds. That is correct, just that Qt5Test is being searched for, so your rationale for applying the patch is moot. Matthew Dawson wrote: Sorry, I misread the patch. You are correct the build is fine as is. We should discuss this on the mailing list first before applying. The Plasma community should also probably come to a consensus for its packages as well. In Plasma most packages
Jenkins build is still unstable: plasma-framework_stable_qt5 » All,LINBUILDER #77
See http://build.kde.org/job/plasma-framework_stable_qt5/Variation=All,label=LINBUILDER/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[attica] /: Correct the url to the OCS specification for the front page at least.
Git commit 37579ccbe9b0a7ef3a7003e975d940970a51092d by Ben Cooksley. Committed on 23/01/2015 at 21:32. Pushed by bcooksley into branch 'master'. Correct the url to the OCS specification for the front page at least. Also, add a giant warning noting that this library is effectively unmaintained. CCMAIL: kde-frameworks-devel@kde.org M +6-1README.md http://commits.kde.org/attica/37579ccbe9b0a7ef3a7003e975d940970a51092d diff --git a/README.md b/README.md index f348438..00fe0c4 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,15 @@ # Attica +## WARNING + +This library is defacto unmaintained based on inaction from the last known major contributors to this library. +Links to outside resources within the documentation may be broken as a result. + ## Introduction Attica is a Qt library that implements the Open Collaboration Services API version 1.6. The REST API is defined here: -http://freedesktop.org/wiki/Specifications/open-collaboration-services-draft/ +http://freedesktop.org/wiki/Specifications/open-collaboration-services/ It grants easy access to the services such as querying information about persons and contents. The library is used in KNewStuff3 as content provider. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Build failed in Jenkins: karchive_stable_qt5 #9
See http://build.kde.org/job/karchive_stable_qt5/9/changes
Changes:
[bhush94] Avoid creating temporary files in current workdir
--
Started by remote host 2a01:4f8:160:9363::9 with note: Triggered by commit
Building remotely on LinuxSlave - 4 (PACKAGER LINBUILDER) in workspace
http://build.kde.org/job/karchive_stable_qt5/ws/
Running Prebuild steps
[karchive_stable_qt5] $ /bin/sh -xe /tmp/hudson9104691858143980521.sh
+ /home/jenkins/scripts/setup-env.sh
Preparing to perform KDE Continuous Integration build
== Setting Up Sources
From git://anongit.kde.org/karchive
1b9772b..98b5a1a master - origin/master
Branch jenkins set up to track remote branch master from origin.
== Cleaning Source Tree
HEAD is now at 1b9772b Add missing QIODevice include
Removing build/
Removing dotdata/
Removing local-inst/
Success build forhudson.tasks.Shell@7b71833f
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/karchive # timeout=10
Fetching upstream changes from git://anongit.kde.org/karchive
git --version # timeout=10
git -c core.askpass=true fetch --tags --progress
git://anongit.kde.org/karchive +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 98b5a1a8d067b684d104692b3477a12bb631f50d
(refs/heads/jenkins)
git config core.sparsecheckout # timeout=10
git checkout -f 98b5a1a8d067b684d104692b3477a12bb631f50d
git rev-list 1b9772b1f801285689612207119e30d0b10fae00 # timeout=10
git tag -a -f -m Jenkins Build #9 jenkins-karchive_stable_qt5-9 # 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]
[karchive_stable_qt5] $ /bin/sh -xe /tmp/hudson5585444365671413374.sh
+ /home/jenkins/scripts/execute-job.sh
KDE Continuous Integration Build
== Building Project: karchive - Branch master
== Build Dependencies:
extra-cmake-modules - Branch master
qt5 - Branch 5.3.2
dogtail - Branch master
cmake - 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 ZLIB: /usr/lib64/libz.so (found version 1.2.7)
-- Found BZip2: /usr/lib64/libbz2.so (found version 1.0.6)
-- Looking for BZ2_bzCompressInit in /usr/lib64/libbz2.so
-- Looking for BZ2_bzCompressInit in /usr/lib64/libbz2.so - found
-- Looking for lzma_auto_decoder in /usr/lib64/liblzma.so
-- Looking for lzma_auto_decoder in /usr/lib64/liblzma.so - found
-- Looking for lzma_easy_encoder in /usr/lib64/liblzma.so
-- Looking for lzma_easy_encoder in /usr/lib64/liblzma.so - found
-- Looking for lzma_lzma_preset in /usr/lib64/liblzma.so
-- Looking for lzma_lzma_preset in /usr/lib64/liblzma.so - found
-- Found LibLZMA: /usr/include (found version 5.0.4)
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
--
-- The following OPTIONAL packages have been found:
* LibLZMA , Support for xz compressed files and data streams ,
http://tukaani.org/xz/
Support for xz compressed files and data streams
-- The following RECOMMENDED packages have been found:
* BZip2 , Support for BZip2 compressed files and data streams ,
http://www.bzip.org
Support for BZip2 compressed files and data streams
-- The following REQUIRED packages have been found:
* ECM (required version = 1.6.0)
* Qt5Core (required version = 5.2.0)
* ZLIB , Support for gzip compressed files and data streams ,
http://www.zlib.net
Required by the core KDE libraries and some criti cal kioslaves
-- Configuring done
-- Generating done
CMake Warning:
Manually-specified variables were not used by
Re: Review Request 122216: Avoid creating temporary files in current workdir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122216/ --- (Updated Jan. 23, 2015, 1:34 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Repository: karchive Description --- KTemporaryFile created temporary files in tmpdir but QTemporaryFile creates temporary file in current working dir. Due to this opening tar.gz or tar.xz archives in Ark or tar kioslave creates ktar-XX.tar file in current working directory in KF5 Diffs - src/ktar.cpp 0972d76 Diff: https://git.reviewboard.kde.org/r/122216/diff/ Testing --- no longer created in $PWD Thanks, Bhushan Shah ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Build failed in Jenkins: karchive_master_qt5 #94
See http://build.kde.org/job/karchive_master_qt5/94/changes
Changes:
[bhush94] Avoid creating temporary files in current workdir
--
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/karchive_master_qt5/ws/
Running Prebuild steps
[karchive_master_qt5] $ /bin/sh -xe /tmp/hudson666905031585947350.sh
+ /home/jenkins/scripts/setup-env.sh
Preparing to perform KDE Continuous Integration build
== Setting Up Sources
From git://anongit.kde.org/karchive
1b9772b..98b5a1a master - origin/master
Branch jenkins set up to track remote branch master from origin.
== Cleaning Source Tree
HEAD is now at 1b9772b Add missing QIODevice include
Removing build/
Removing dotdata/
Removing local-inst/
Success build forhudson.tasks.Shell@1360dd2e
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/karchive # timeout=10
Fetching upstream changes from git://anongit.kde.org/karchive
git --version # timeout=10
git -c core.askpass=true fetch --tags --progress
git://anongit.kde.org/karchive +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 98b5a1a8d067b684d104692b3477a12bb631f50d
(refs/heads/jenkins)
git config core.sparsecheckout # timeout=10
git checkout -f 98b5a1a8d067b684d104692b3477a12bb631f50d
git rev-list 1b9772b1f801285689612207119e30d0b10fae00 # timeout=10
git tag -a -f -m Jenkins Build #94 jenkins-karchive_master_qt5-94 #
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]
[karchive_master_qt5] $ /bin/sh -xe /tmp/hudson3454693978458611634.sh
+ /home/jenkins/scripts/execute-job.sh
KDE Continuous Integration Build
== Building Project: karchive - Branch master
== Build Dependencies:
extra-cmake-modules - Branch master
cmake - Branch master
dogtail - Branch master
qt5 - Branch 5.4.0
== 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 ZLIB: /usr/lib64/libz.so (found version 1.2.7)
-- Found BZip2: /usr/lib64/libbz2.so (found version 1.0.6)
-- Looking for BZ2_bzCompressInit in /usr/lib64/libbz2.so
-- Looking for BZ2_bzCompressInit in /usr/lib64/libbz2.so - found
-- Looking for lzma_auto_decoder in /usr/lib64/liblzma.so
-- Looking for lzma_auto_decoder in /usr/lib64/liblzma.so - found
-- Looking for lzma_easy_encoder in /usr/lib64/liblzma.so
-- Looking for lzma_easy_encoder in /usr/lib64/liblzma.so - found
-- Looking for lzma_lzma_preset in /usr/lib64/liblzma.so
-- Looking for lzma_lzma_preset in /usr/lib64/liblzma.so - found
-- Found LibLZMA: /usr/include (found version 5.0.4)
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
--
-- The following OPTIONAL packages have been found:
* LibLZMA , Support for xz compressed files and data streams ,
http://tukaani.org/xz/
Support for xz compressed files and data streams
-- The following RECOMMENDED packages have been found:
* BZip2 , Support for BZip2 compressed files and data streams ,
http://www.bzip.org
Support for BZip2 compressed files and data streams
-- The following REQUIRED packages have been found:
* ECM (required version = 1.6.0)
* Qt5Core (required version = 5.2.0)
* ZLIB , Support for gzip compressed files and data streams ,
http://www.zlib.net
Required by the core KDE libraries and some criti cal kioslaves
-- Configuring done
-- Generating done
CMake Warning:
Manually-specified variables were not used
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. - Vishesh Handa On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ 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 : karchive_master_qt5 #95
See http://build.kde.org/job/karchive_master_qt5/95/changes ___ 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 : karchive_stable_qt5 #10
See http://build.kde.org/job/karchive_stable_qt5/10/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122210: KCmdLineArgs: Fix -version handling
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122210/#review74575 --- src/kdecore/kcmdlineargs.cpp https://git.reviewboard.kde.org/r/122210/#comment51709 Should be KDE Frameworks now, IMHO. - David Faure On Jan. 22, 2015, 8:57 p.m., Albert Astals Cid wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122210/ --- (Updated Jan. 22, 2015, 8:57 p.m.) Review request for KDE Frameworks. Repository: kdelibs4support Description --- Remove the TODO and fix the i18n usage Diffs - src/kdecore/kcmdlineargs.cpp de2f829 Diff: https://git.reviewboard.kde.org/r/122210/diff/ Testing --- konsole -v is no longer ugly as hell Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122204: Mark results as required.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/#review74576 --- Ship it! It if just warns, then sure. I assume there's a -Werror= to make it error if desired? Would be good to do a full kdesrc-build with that set ;) - David Faure On Jan. 22, 2015, 6:34 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122204/ --- (Updated Jan. 22, 2015, 6:34 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This prevents API misuage, similar to how QString::arg is doing it. See also: http://quickgit.kde.org/?p=kdevplatform.gita=commith=078f1cd584e2de75ad19c46282b47dd1e0feff8f Diffs - src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 Diff: https://git.reviewboard.kde.org/r/122204/diff/ Testing --- Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122216: Avoid creating temporary files in current workdir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122216/ --- Review request for KDE Frameworks and David Faure. Repository: karchive Description --- KTemporaryFile created temporary files in tmpdir but QTemporaryFile creates temporary file in current working dir. Due to this opening tar.gz or tar.xz archives in Ark or tar kioslave creates ktar-XX.tar file in current working directory in KF5 Diffs - src/ktar.cpp 0972d76 Diff: https://git.reviewboard.kde.org/r/122216/diff/ Testing --- no longer created in $PWD Thanks, Bhushan Shah ___ 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/ --- (Updated Jan. 23, 2015, 12:10 p.m.) Status -- This change has been marked as submitted. 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 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/ --- (Updated Jan. 23, 2015, 12:10 p.m.) Status -- This change has been marked as submitted. 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
Build failed in Jenkins: kde-baseapps_master_qt5 #207
See http://build.kde.org/job/kde-baseapps_master_qt5/207/changes Changes: [tittiatcoke] Install icons in the right icon theme directory -- [...truncated 2912 lines...] ^ http://build.kde.org/job/kde-baseapps_master_qt5/ws/konqueror/src/tests/konqviewmgrtest.cpp:894:60: warning: ‘KTabWidget’ is deprecated (declared at /srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/ktabwidget.h:44) [-Wdeprecated-declarations] KTabWidget* tabWidget = mainWindow.findChildKTabWidget*(); ^ Linking CXX executable undomanagertest Linking CXX executable konqhtmltest Linking CXX executable konqviewtest [ 64%] Built target undomanagertest [ 64%] Built target konqhtmltest [ 64%] Built target dolphinprivate [ 64%] [ 64%] Generating dolphin_folderspanelsettings.h, dolphin_folderspanelsettings.cpp Generating dolphin_searchsettings.h, dolphin_searchsettings.cpp [ 64%] Generating dolphin_informationpanelsettings.h, dolphin_informationpanelsettings.cpp [ 64%] Built target konqviewtest [ 64%] Generating dolphin_placespanelsettings.h, dolphin_placespanelsettings.cpp [ 64%] Generating dolphin_searchsettings.h, dolphin_searchsettings.cpp Scanning dependencies of target kcm_dolphinnavigation [ 64%] Linking CXX executable konqviewmgrtest Scanning dependencies of target dolphinpart Building CXX object dolphin/src/CMakeFiles/kcm_dolphinnavigation.dir/settings/kcm/kcmdolphinnavigation.cpp.o [ 64%] Building CXX object dolphin/src/CMakeFiles/dolphinpart.dir/dolphinpart.cpp.o Scanning dependencies of target kcm_dolphinservices Scanning dependencies of target dolphinsearchboxtest [ 64%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinservices.dir/settings/kcm/kcmdolphinservices.cpp.o [ 64%] Building CXX object dolphin/src/tests/CMakeFiles/dolphinsearchboxtest.dir/dolphinsearchboxtest.cpp.o Scanning dependencies of target kcm_dolphinviewmodes Scanning dependencies of target kcm_dolphingeneral [ 64%] [ 64%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinviewmodes.dir/settings/kcm/kcmdolphinviewmodes.cpp.o Building CXX object dolphin/src/CMakeFiles/kcm_dolphingeneral.dir/settings/kcm/kcmdolphingeneral.cpp.o [ 64%] Built target konqviewmgrtest [ 64%] Scanning dependencies of target kfileitemlistviewtest Building CXX object dolphin/src/CMakeFiles/kcm_dolphingeneral.dir/settings/general/behaviorsettingspage.cpp.o [ 64%] Building CXX object dolphin/src/tests/CMakeFiles/kfileitemlistviewtest.dir/kfileitemlistviewtest.cpp.o [ 64%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinnavigation.dir/settings/navigation/navigationsettingspage.cpp.o [ 64%] Building CXX object dolphin/src/tests/CMakeFiles/dolphinsearchboxtest.dir/__/search/dolphinfacetswidget.cpp.o [ 65%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinservices.dir/settings/services/servicessettingspage.cpp.o [ 65%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinservices.dir/settings/settingspagebase.cpp.o Scanning dependencies of target kdeinit_dolphin [ 66%] Building CXX object dolphin/src/CMakeFiles/kdeinit_dolphin.dir/dolphinapplication.cpp.o http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/general/behaviorsettingspage.cpp: In member function ‘virtual void BehaviorSettingsPage::applySettings()’: http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/general/behaviorsettingspage.cpp:116:75: warning: ‘static void KGlobalSettings::emitChange(KGlobalSettings::ChangeType, int)’ is deprecated (declared at /srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kglobalsettings.h:559) [-Wdeprecated-declarations] KGlobalSettings::emitChange(KGlobalSettings::NaturalSortingChanged); ^ [ 66%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphinviewmodes.dir/settings/viewmodes/dolphinfontrequester.cpp.o [ 66%] Building CXX object dolphin/src/CMakeFiles/kcm_dolphingeneral.dir/settings/general/previewssettingspage.cpp.o http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/navigation/navigationsettingspage.cpp: In member function ‘virtual void NavigationSettingsPage::applySettings()’: http://build.kde.org/job/kde-baseapps_master_qt5/ws/dolphin/src/settings/navigation/navigationsettingspage.cpp:91:106: warning: ‘static void KGlobalSettings::emitChange(KGlobalSettings::ChangeType, int)’ is deprecated (declared at /srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kglobalsettings.h:559) [-Wdeprecated-declarations] KGlobalSettings::self()-emitChange(KGlobalSettings::SettingsChanged, KGlobalSettings::SETTINGS_MOUSE); ^
Re: Review Request 122216: Avoid creating temporary files in current workdir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122216/#review74584 --- Ship it! Ship It! - David Faure On Jan. 23, 2015, 8:44 a.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122216/ --- (Updated Jan. 23, 2015, 8:44 a.m.) Review request for KDE Frameworks and David Faure. Repository: karchive Description --- KTemporaryFile created temporary files in tmpdir but QTemporaryFile creates temporary file in current working dir. Due to this opening tar.gz or tar.xz archives in Ark or tar kioslave creates ktar-XX.tar file in current working directory in KF5 Diffs - src/ktar.cpp 0972d76 Diff: https://git.reviewboard.kde.org/r/122216/diff/ Testing --- no longer created in $PWD Thanks, Bhushan Shah ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122220: Don't show two progress dialogs for PasteJob
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/ --- Review request for KDE Frameworks and David Faure. Bugs: 342276 https://bugs.kde.org/show_bug.cgi?id=342276 Repository: kio Description --- PasteJob creates a copy/move job and passes its flags to it. These jobs then register with the progress dialog, but so does the original PasteJob and then there are two progress bars/dialogs. This moves the registration to the job tracker only to the pasteMimeDataImpl job which does not take the flags of the parent job. I'm not sure if it makes sense there though. Alternatively it could be just removed. Diffs - src/widgets/pastejob.cpp a0ce3ac src/widgets/pastejob_p.h f5e68d3 Diff: https://git.reviewboard.kde.org/r/10/diff/ Testing --- Copying and moving files no longer spawns two progress dialogs. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 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/ --- (Updated Jan. 23, 2015, 2:17 p.m.) Review request for KDE Frameworks. Changes --- This patch actually makes the test fail without changing the default values (another patch on reviewboard) 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 (updated) - autotests/convertertest.h 8129a48 autotests/convertertest.cpp ae4298e src/currency.cpp 715233c 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
Re: Review Request 122220: Don't show two progress dialogs for PasteJob
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/#review74603 --- src/widgets/pastejob.cpp https://git.reviewboard.kde.org/r/10/#comment51713 Not needed. This is up to the subjob itself. It in fact already does so, since we don't pass HideProgressInfo to it. (cf putDataAsyncTo in paste.cpp) src/widgets/pastejob_p.h https://git.reviewboard.kde.org/r/10/#comment51712 Correct, but very unusual for kio jobs, so please add a comment, like this: // Note: never KIO::getJobTracker()-registerJob here // The progress information comes from the underlying job (so we don't have to forward it here). (This way, if we ever find a reason why this job should be registered and not the subjob, we'll know that it's possible, provided that we forward progress info here). One reason could be - does everything work correctly when the user clicks Cancel in the progress dialog. I think so, but I didn't test (mostly tested this with unittests). Anyway - I want the comment in here so that if anyone uses this as the starting point for a new job, then that person remembers to think about whether or not to call registerJob ;-) Thanks for pinpointing the issue! - David Faure On Jan. 23, 2015, 2:13 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/ --- (Updated Jan. 23, 2015, 2:13 p.m.) Review request for KDE Frameworks and David Faure. Bugs: 342276 https://bugs.kde.org/show_bug.cgi?id=342276 Repository: kio Description --- PasteJob creates a copy/move job and passes its flags to it. These jobs then register with the progress dialog, but so does the original PasteJob and then there are two progress bars/dialogs. This moves the registration to the job tracker only to the pasteMimeDataImpl job which does not take the flags of the parent job. I'm not sure if it makes sense there though. Alternatively it could be just removed. Diffs - src/widgets/pastejob.cpp a0ce3ac src/widgets/pastejob_p.h f5e68d3 Diff: https://git.reviewboard.kde.org/r/10/diff/ Testing --- Copying and moving files no longer spawns two progress dialogs. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122220: Don't show two progress dialogs for PasteJob
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/ --- (Updated Jan. 23, 2015, 5:15 p.m.) Review request for KDE Frameworks and David Faure. Bugs: 342276 https://bugs.kde.org/show_bug.cgi?id=342276 Repository: kio Description --- PasteJob creates a copy/move job and passes its flags to it. These jobs then register with the progress dialog, but so does the original PasteJob and then there are two progress bars/dialogs. This moves the registration to the job tracker only to the pasteMimeDataImpl job which does not take the flags of the parent job. I'm not sure if it makes sense there though. Alternatively it could be just removed. Diffs (updated) - src/widgets/pastejob_p.h f5e68d3 Diff: https://git.reviewboard.kde.org/r/10/diff/ Testing --- Copying and moving files no longer spawns two progress dialogs. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122220: Don't show two progress dialogs for PasteJob
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/#review74611 --- Ship it! (make sure to update the commit log too) - David Faure On Jan. 23, 2015, 4:15 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/ --- (Updated Jan. 23, 2015, 4:15 p.m.) Review request for KDE Frameworks and David Faure. Bugs: 342276 https://bugs.kde.org/show_bug.cgi?id=342276 Repository: kio Description --- PasteJob creates a copy/move job and passes its flags to it. These jobs then register with the progress dialog, but so does the original PasteJob and then there are two progress bars/dialogs. This moves the registration to the job tracker only to the pasteMimeDataImpl job which does not take the flags of the parent job. I'm not sure if it makes sense there though. Alternatively it could be just removed. Diffs - src/widgets/pastejob_p.h f5e68d3 Diff: https://git.reviewboard.kde.org/r/10/diff/ Testing --- Copying and moving files no longer spawns two progress dialogs. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122220: Don't show two progress dialogs for PasteJob
On Jan. 23, 2015, 3:40 p.m., David Faure wrote: src/widgets/pastejob_p.h, line 58 https://git.reviewboard.kde.org/r/10/diff/1/?file=344358#file344358line58 Correct, but very unusual for kio jobs, so please add a comment, like this: // Note: never KIO::getJobTracker()-registerJob here // The progress information comes from the underlying job (so we don't have to forward it here). (This way, if we ever find a reason why this job should be registered and not the subjob, we'll know that it's possible, provided that we forward progress info here). One reason could be - does everything work correctly when the user clicks Cancel in the progress dialog. I think so, but I didn't test (mostly tested this with unittests). Anyway - I want the comment in here so that if anyone uses this as the starting point for a new job, then that person remembers to think about whether or not to call registerJob ;-) does everything work correctly when the user clicks Cancel in the progress dialog Clicking cancel on the subjob does cancel it properly (the file will stay half-written on copy, but I'm not sure if that's wanted or not), but Plasma will not spawn any notification while it does when cancelling the PasteJob (although it says Copying Finished which is kinda wrong anyway). But that's a bug in the Plasma applet, so I'll have a look at that too. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/#review74603 --- On Jan. 23, 2015, 3:13 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/10/ --- (Updated Jan. 23, 2015, 3:13 p.m.) Review request for KDE Frameworks and David Faure. Bugs: 342276 https://bugs.kde.org/show_bug.cgi?id=342276 Repository: kio Description --- PasteJob creates a copy/move job and passes its flags to it. These jobs then register with the progress dialog, but so does the original PasteJob and then there are two progress bars/dialogs. This moves the registration to the job tracker only to the pasteMimeDataImpl job which does not take the flags of the parent job. I'm not sure if it makes sense there though. Alternatively it could be just removed. Diffs - src/widgets/pastejob.cpp a0ce3ac src/widgets/pastejob_p.h f5e68d3 Diff: https://git.reviewboard.kde.org/r/10/diff/ Testing --- Copying and moving files no longer spawns two progress dialogs. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
