Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 6:07 p.m.) Review request for KDE Frameworks and Matthew Dawson. Changes --- * Create a different kind of signal that will be generated whenever the set method is called for a property, in contrast to whenever it's written to disk. * Don't consider properties as modified if the same value is set. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs (updated) - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 6:13 p.m.) Review request for KDE Frameworks. Changes --- Fix unit test as pointed out by Albert Vaca, still passes. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs (updated) - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79346 --- autotests/kconfig_compiler/test13.h.ref (line 38) https://git.reviewboard.kde.org/r/123367/#comment54192 I'm not sure if brightnessChanged is the right signal to use here, as calling setBrightness won't trigger it. I'm not sure if users would expect it to change then, or when this object actually gets saved. Thoughts? src/kconfig_compiler/kconfig_compiler.cpp (line 100) https://git.reviewboard.kde.org/r/123367/#comment54191 Is there a reason not to generate Q_PROPERTIES for all classes, or at least do it by default? This seems like a useful thing to have every class use, so adding another configuration only reduces its visibility. - Matthew Dawson On April 22, 2015, 10:51 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 10:51 a.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
On April 22, 2015, 4:36 p.m., Vishesh Handa wrote: autotests/kconfig_compiler/test_signal.h.ref, line 126 https://git.reviewboard.kde.org/r/123367/diff/3/?file=361167#file361167line126 Is the move required? This is a unit test, this is how it gets generated after the patch. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79343 --- On April 15, 2015, 5:38 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 15, 2015, 5:38 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 4:51 p.m.) Review request for KDE Frameworks and Matthew Dawson. Changes --- Only initialize the hasSignals variable once we know if there's signals (after going through all the properties which might add it). Add the Q_OBJECT macro whether there's Q_PROPERTY or Q_SIGNAL. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs (updated) - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
On April 22, 2015, 9 a.m., Emmanuel Pescosta wrote: src/widgets/accessmanager.cpp, line 255 https://git.reviewboard.kde.org/r/123443/diff/1/?file=362233#file362233line255 IMO you should always verify if the data is readable, because not all lib users are trustworthy (assert isn't meant for input validation of untrustworthy callers - public api) Aleix Pol Gonzalez wrote: I can do it, but QNetworkAccessManager doesn't do that, and I'd say it's better to be as similar to QNAM as possible. Ah ok sry for the noice, didn't know that. Just assumed that they validate it because they mention data must be opened for reading in their API docs. - Emmanuel --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79325 --- On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 1:46 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79343 --- autotests/kconfig_compiler/test_signal.h.ref (line 121) https://git.reviewboard.kde.org/r/123367/#comment54187 Is the move required? src/kconfig_compiler/kconfig_compiler.cpp (line 1722) https://git.reviewboard.kde.org/r/123367/#comment54188 It might be more readable, if you created a new variable called 'requiresMoc' or 'requiresQObjectMacro'. - Vishesh Handa On April 15, 2015, 3:38 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 15, 2015, 3:38 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
On April 22, 2015, 10:53 a.m., Matthew Dawson wrote: src/kconfig_compiler/kconfig_compiler.cpp, line 100 https://git.reviewboard.kde.org/r/123367/diff/3/?file=361168#file361168line100 Is there a reason not to generate Q_PROPERTIES for all classes, or at least do it by default? This seems like a useful thing to have every class use, so adding another configuration only reduces its visibility. Aleix Pol Gonzalez wrote: To minimize changes. I agree it would be interesting, I can do it if you (collectively) are on board. Do you have an idea of what should the setting be called instead? DoNotGenerateProperties? I'd just change the default to be true. Based upon what I remember about UX, double negatives should be avoided. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79346 --- On April 22, 2015, 10:51 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 10:51 a.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79347 --- Ship it! - Vishesh Handa On April 22, 2015, 2:51 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 2:51 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
On April 22, 2015, 4:53 p.m., Matthew Dawson wrote: src/kconfig_compiler/kconfig_compiler.cpp, line 100 https://git.reviewboard.kde.org/r/123367/diff/3/?file=361168#file361168line100 Is there a reason not to generate Q_PROPERTIES for all classes, or at least do it by default? This seems like a useful thing to have every class use, so adding another configuration only reduces its visibility. To minimize changes. I agree it would be interesting, I can do it if you (collectively) are on board. Do you have an idea of what should the setting be called instead? DoNotGenerateProperties? On April 22, 2015, 4:53 p.m., Matthew Dawson wrote: autotests/kconfig_compiler/test13.h.ref, line 38 https://git.reviewboard.kde.org/r/123367/diff/3/?file=361162#file361162line38 I'm not sure if brightnessChanged is the right signal to use here, as calling setBrightness won't trigger it. I'm not sure if users would expect it to change then, or when this object actually gets saved. Thoughts? That makes sense. I'll look into that. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79346 --- On April 22, 2015, 4:51 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 4:51 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
On April 22, 2015, 4:53 p.m., Matthew Dawson wrote: src/kconfig_compiler/kconfig_compiler.cpp, line 100 https://git.reviewboard.kde.org/r/123367/diff/3/?file=361168#file361168line100 Is there a reason not to generate Q_PROPERTIES for all classes, or at least do it by default? This seems like a useful thing to have every class use, so adding another configuration only reduces its visibility. Aleix Pol Gonzalez wrote: To minimize changes. I agree it would be interesting, I can do it if you (collectively) are on board. Do you have an idea of what should the setting be called instead? DoNotGenerateProperties? Matthew Dawson wrote: I'd just change the default to be true. Based upon what I remember about UX, double negatives should be avoided. Ok, let's do it in a separate iteration of the patch though. This is going to make a huge patch as it will require changing most of the unit tests. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79346 --- On April 22, 2015, 4:51 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 4:51 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
On April 22, 2015, 10:53 a.m., Matthew Dawson wrote: src/kconfig_compiler/kconfig_compiler.cpp, line 100 https://git.reviewboard.kde.org/r/123367/diff/3/?file=361168#file361168line100 Is there a reason not to generate Q_PROPERTIES for all classes, or at least do it by default? This seems like a useful thing to have every class use, so adding another configuration only reduces its visibility. Aleix Pol Gonzalez wrote: To minimize changes. I agree it would be interesting, I can do it if you (collectively) are on board. Do you have an idea of what should the setting be called instead? DoNotGenerateProperties? Matthew Dawson wrote: I'd just change the default to be true. Based upon what I remember about UX, double negatives should be avoided. Aleix Pol Gonzalez wrote: Ok, let's do it in a separate iteration of the patch though. This is going to make a huge patch as it will require changing most of the unit tests. Ok, sounds good. For that patch, it would be good to leave some unit tests not generating properties, to ensure the negative works as well. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79346 --- On April 22, 2015, 10:51 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 10:51 a.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/#review79323 --- src/kidletime.cpp (lines 222 - 223) https://git.reviewboard.kde.org/r/123417/#comment54155 that would cause a crash. It will set the poller to null and afterwards unreference the nullptr. - Martin Gräßlin On April 22, 2015, 3:18 a.m., Nerdopolis Turfwalker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 3:18 a.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79353 --- If the first issue doesn't fit into this commit, this has a ship it from me after the two nitpicks are fixed. autotests/kconfig_compiler/test13.h.ref (line 51) https://git.reviewboard.kde.org/r/123367/#comment54201 This looks better, but I just thought of one other case: when the underlying KConfig class gets updated and the KConfigSkeleton updates. If that isn't too much work to do here, I'd appreciate it if it's done now. Otherwise please file a bug against KConfig so I don't forget. src/kconfig_compiler/kconfig_compiler.cpp (line 1418) https://git.reviewboard.kde.org/r/123367/#comment54202 Nitpick: Please always include braces on if statements. src/kconfig_compiler/kconfig_compiler.cpp (line 1452) https://git.reviewboard.kde.org/r/123367/#comment54203 Nitpick: braces here too. - Matthew Dawson On April 22, 2015, 12:07 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 12:07 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
New framework: KF5Syndication
Hi all, I'd like to ask for review of another Framework from kdepimlibs: KF5Syndication KF5Syndication is an RSS/Atom parsing library. It also provides API to fetch feeds directly from network. It's a Tier 3 Framework (depends on KCodecs and KIO). AFAIK it's currently being used only by Akregator. I would like to submit KF5Syndication for the standard 2 week review period, and if everything is OK, then move it to Frameworks. Cheers, Daniel -- Daniel Vrátil www.dvratil.cz | dvra...@kde.org IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde) ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 23, 2015, 3:20 a.m.) Review request for KDE Frameworks and Matthew Dawson. Changes --- Generate again the Changed signal for each of the properties. Also emit the Modified signal together with the Changed signal. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs (updated) - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79355 --- autotests/kconfig_compiler/test13.h.ref (line 20) https://git.reviewboard.kde.org/r/123367/#comment54204 Why is there no brightness property anymore? - Albert Astals Cid On abr. 22, 2015, 4:07 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated abr. 22, 2015, 4:07 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
On April 22, 2015, 5 p.m., Albert Astals Cid wrote: autotests/kconfig_compiler/test13.h.ref, line 20 https://git.reviewboard.kde.org/r/123367/diff/5/?file=362514#file362514line20 Why is there no brightness property anymore? I still see it on line 40. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79355 --- On April 22, 2015, 12:07 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated April 22, 2015, 12:07 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: New framework: KF5Syndication
On 22 Apr 2015, at 22:51, Mark Gaiser mark...@gmail.com wrote: Disclaimer: I've never seen this code nor knew about it's existence till ~30 minutes ago. This library depends on QtXml (quite heavily in fact). That very Qt module is deprecated [1]. Funnily enough, that is the only place that mentions it as being deprecated. No QtXml classes mention it in the docs and you won't get any compiler warnings either when you decide to use it. True, the heavy QDom usage is a burden, which I don’t think you can get rid of in a sensible way, not without changing both API and implementation to a point where you end up with a rewrite, not a refactoring. If I would rewrite such a library today, I’d use QXmlStreamReader instead of QDom. I’d also get rid of all that funky polymorphism and design pattern-isms, and just parse the feed data from the XML and create plain and dumb value-type objects for feeds, items etc. (and kill it’s homegrown RDF parser/model…) Frank ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: New framework: KF5Syndication
On Thu, Apr 23, 2015 at 12:13 AM, Daniel Vrátil dvra...@kde.org wrote: On Wednesday, April 22, 2015 11:02:31 PM Frank Osterfeld wrote: Hi, KF5Syndication is an RSS/Atom parsing library. It also provides API to fetch feeds directly from network. It's a Tier 3 Framework (depends on KCodecs and KIO). AFAIK it's currently being used only by Akregator. The Akonadi RSS resources (still lingering in some work branch, I assume?) and some plasmoids were also using it. As for the dependencies: The KIO dependency is used only by a single class, “DataRetriever”, which is a rather thin wrapper around KIO::get to download the feed file. The actual parser only requires KCodecs from KF. So I wonder if it would make sense to either drop the retriever classes completely, split them into another library (too tiny I guess), or make them optional in the build. If KIO is really needed only to fetch the feeds then I would say we can drop it. If Akregator ever gets ported to Akonadi :), then the retrieval will be handled by the resource and other applications using Syndication will probably prefer their own retrieval methods and can always fall back to just calling KIO::get themselves. Dan Frank -- Daniel Vrátil www.dvratil.cz | dvra...@kde.org IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde) GPG Key: 0x4D69557AECB13683 Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel As a hint, for KDE Connect what I'm working on is to use KIO::AccessManager so in practice the framework only depends on QtNetwork (QNetworkAccessManager) and then a hook is provided to feed the KIO implementation. This is also what QtWebKit does. HTH, Aleix ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: New framework: KF5Syndication
On Wed, Apr 22, 2015 at 9:44 PM, Daniel Vrátil dvra...@kde.org wrote: Hi all, I'd like to ask for review of another Framework from kdepimlibs: KF5Syndication KF5Syndication is an RSS/Atom parsing library. It also provides API to fetch feeds directly from network. It's a Tier 3 Framework (depends on KCodecs and KIO). AFAIK it's currently being used only by Akregator. I would like to submit KF5Syndication for the standard 2 week review period, and if everything is OK, then move it to Frameworks. Cheers, Daniel Disclaimer: I've never seen this code nor knew about it's existence till ~30 minutes ago. This library depends on QtXml (quite heavily in fact). That very Qt module is deprecated [1]. Funnily enough, that is the only place that mentions it as being deprecated. No QtXml classes mention it in the docs and you won't get any compiler warnings either when you decide to use it. When searching for bug reports for deprecating QtXml i found [2] where they say it's been mislabeled and should have the same status as Widgets. Aka: done. Which also means it might go deprecated in some future release (Qt6?). Now i don't know what the real state of QtXml is, but if it really is deprecated then i don't know if it's wise to release this library while depending on a deprecated module. In that case it might be better to port (which probably requires quite some work) to the QXmlStreamReader and QXmlStreamWriter. Just my 5 cents :) [1] http://doc.qt.io/qt-5/qtmodules.html [2] https://bugreports.qt.io/browse/QTBUG-32926 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
kf5-qt5 compile failures, need developer assistance
These are from the kf5-qt5 branchGroup (usually master or frameworks branches) I realize many these are still under heavy development for porting. This is just a note to inform. Fixes welcome though. kcalcore: https://build-sandbox.kde.org/job/kcalcore%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/9/console libechonest: qjson does not exist in qt5 world, code must adapt to use qt5 json support https://build-sandbox.kde.org/job/libechonest%20qt5%20kf5-qt5/PLATFORM=Linux,compiler=gcc/9/console kget: It looks like it is looking for qt4 varients of qca and qgpgmepp Should be qca_qt5 and gpgmepp (no longer bundled in kdepimlibs) https://build-sandbox.kde.org/job/kget%20kf5_port%20kf5-qt5/PLATFORM=Linux,compiler=gcc/15/console choqok can't find qoauth in lib64 https://build-sandbox.kde.org/job/choqok%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/11/console marble: (this also breaks libkgeomap and digikam) https://build-sandbox.kde.org/job/marble%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/1/console kdev-go https://build-sandbox.kde.org/job/kdev-go%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/2/console kdev-python https://build-sandbox.kde.org/job/kdev-python%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/10/console tellico: https://build-sandbox.kde.org/job/tellico%20frameworks%20kf5-qt5/PLATFORM=Linux,compiler=gcc/13/console calligra: https://build-sandbox.kde.org/job/calligra%20frameworks%20kf5-qt5/lastFailedBuild/console kile: https://build-sandbox.kde.org/job/kile%20frameworks%20kf5-qt5/PLATFORM=Linux,compiler=gcc/17/ smokeqt: https://build-sandbox.kde.org/job/smokeqt%20Qt5%20kf5-qt5/PLATFORM=Linux,compiler=gcc/9/console libringclient: https://build-sandbox.kde.org/job/libringclient%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/10/console phonon-gstreamer: https://build-sandbox.kde.org/job/phonon-gstreamer%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/7/console plasma-nm: https://build-sandbox.kde.org/job/plasma-nm%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/12/console wacomtablet: https://build-sandbox.kde.org/job/wacomtablet%20kf5-port%20kf5-qt5/PLATFORM=Linux,compiler=gcc/13/console signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123367: Generate Q_PROPERTY entries out of KConfigSkeleton classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/#review79361 --- autotests/kconfig_compiler/test13.h.ref (line 20) https://git.reviewboard.kde.org/r/123367/#comment54213 Lol i'm blind - Albert Astals Cid On abr. 22, 2015, 4:07 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123367/ --- (Updated abr. 22, 2015, 4:07 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- The generation of those classes makes it useful to have these being used within C++ application. This change makes it possible to use these classes from QML as well. For each variable, exposes the getter. In case there's a setter, it will add a notify signal and the setter to the property. Diffs - autotests/kconfig_compiler/CMakeLists.txt 0cca605 autotests/kconfig_compiler/kconfigcompiler_test.cpp 43623ce autotests/kconfig_compiler/test13.cpp.ref PRE-CREATION autotests/kconfig_compiler/test13.h.ref PRE-CREATION autotests/kconfig_compiler/test13.kcfg PRE-CREATION autotests/kconfig_compiler/test13.kcfgc PRE-CREATION autotests/kconfig_compiler/test13main.cpp PRE-CREATION autotests/kconfig_compiler/test_signal.cpp.ref 58e73ef autotests/kconfig_compiler/test_signal.h.ref 19b8b40 src/kconfig_compiler/kconfig_compiler.cpp 5aae340 Diff: https://git.reviewboard.kde.org/r/123367/diff/ Testing --- KConfig tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: New framework: KF5Syndication
Hi, KF5Syndication is an RSS/Atom parsing library. It also provides API to fetch feeds directly from network. It's a Tier 3 Framework (depends on KCodecs and KIO). AFAIK it's currently being used only by Akregator. The Akonadi RSS resources (still lingering in some work branch, I assume?) and some plasmoids were also using it. As for the dependencies: The KIO dependency is used only by a single class, “DataRetriever”, which is a rather thin wrapper around KIO::get to download the feed file. The actual parser only requires KCodecs from KF. So I wonder if it would make sense to either drop the retriever classes completely, split them into another library (too tiny I guess), or make them optional in the build. Frank ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: New framework: KF5Syndication
On Wednesday, April 22, 2015 10:51:15 PM Mark Gaiser wrote: On Wed, Apr 22, 2015 at 9:44 PM, Daniel Vrátil dvra...@kde.org wrote: Hi all, I'd like to ask for review of another Framework from kdepimlibs: KF5Syndication KF5Syndication is an RSS/Atom parsing library. It also provides API to fetch feeds directly from network. It's a Tier 3 Framework (depends on KCodecs and KIO). AFAIK it's currently being used only by Akregator. I would like to submit KF5Syndication for the standard 2 week review period, and if everything is OK, then move it to Frameworks. Cheers, Daniel Disclaimer: I've never seen this code nor knew about it's existence till ~30 minutes ago. This library depends on QtXml (quite heavily in fact). That very Qt module is deprecated [1]. Funnily enough, that is the only place that mentions it as being deprecated. No QtXml classes mention it in the docs and you won't get any compiler warnings either when you decide to use it. When searching for bug reports for deprecating QtXml i found [2] where they say it's been mislabeled and should have the same status as Widgets. Aka: done. Which also means it might go deprecated in some future release (Qt6?). Now i don't know what the real state of QtXml is, but if it really is deprecated then i don't know if it's wise to release this library while depending on a deprecated module. In that case it might be better to port (which probably requires quite some work) to the QXmlStreamReader and QXmlStreamWriter. I don't think we need to bother ourselves with this now since QtXml is not going away in Qt 5. If QtXml is indeed removed from Qt at some point then we will of course have to re-evaluate switching to QXmlStreamReader (or some 3rd party parser). QtXml is not exposed in public API of Syndication, so we should be able to just switch to different parser in KF6 without breaking API (too much). QXmlStreamReader has a rather low-level API IMO - it's OK if you want to parse the XML top-down but what Syndication does is closer to mapping objects to parts of the XML DOM so as you said rewriting it to QXmlStreamReader would be quite a lot of work. Cheers, Dan Just my 5 cents :) [1] http://doc.qt.io/qt-5/qtmodules.html [2] https://bugreports.qt.io/browse/QTBUG-32926 -- Daniel Vrátil www.dvratil.cz | dvra...@kde.org IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde) GPG Key: 0x4D69557AECB13683 Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683 signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: New framework: KF5Syndication
On Wednesday, April 22, 2015 11:02:31 PM Frank Osterfeld wrote: Hi, KF5Syndication is an RSS/Atom parsing library. It also provides API to fetch feeds directly from network. It's a Tier 3 Framework (depends on KCodecs and KIO). AFAIK it's currently being used only by Akregator. The Akonadi RSS resources (still lingering in some work branch, I assume?) and some plasmoids were also using it. As for the dependencies: The KIO dependency is used only by a single class, “DataRetriever”, which is a rather thin wrapper around KIO::get to download the feed file. The actual parser only requires KCodecs from KF. So I wonder if it would make sense to either drop the retriever classes completely, split them into another library (too tiny I guess), or make them optional in the build. If KIO is really needed only to fetch the feeds then I would say we can drop it. If Akregator ever gets ported to Akonadi :), then the retrieval will be handled by the resource and other applications using Syndication will probably prefer their own retrieval methods and can always fall back to just calling KIO::get themselves. Dan Frank -- Daniel Vrátil www.dvratil.cz | dvra...@kde.org IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde) GPG Key: 0x4D69557AECB13683 Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683 signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 123463: Use categorized logging in runtime component
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123463/ --- Review request for KDE Frameworks and Martin Klapetek. Bugs: 346429 https://bugs.kde.org/show_bug.cgi?id=346429 Repository: kglobalaccel Description --- BUG: 346429 Diffs - src/runtime/CMakeLists.txt e639fa5df96859f844476492235909756220ae5d src/runtime/component.cpp 3b5bdf99d0d26c032f2315bc4c1e64de5d321ee7 src/runtime/globalshortcut.cpp 3fe7bd961d3cdb23857654b3abff630d5ec0c3ec src/runtime/globalshortcutsregistry.cpp 3e4d720e13f6cda8e1b2827c70fce22423474d75 src/runtime/kglobalaccel_mac.cpp daaa24c3ec75cf6e78c923083b824ff331d6fe75 src/runtime/kglobalaccel_win.cpp 7457e348502b6f9d8cbb71be37652cf10ecf6587 src/runtime/kglobalaccel_x11.cpp abee5bcaa9795436833bc0b7fa5b259e7c3f86ac src/runtime/kglobalacceld.cpp 4e7cb9df1da37311d6f9e098f4d5b217cad54fd9 src/runtime/logging.cpp PRE-CREATION src/runtime/logging_p.h PRE-CREATION src/runtime/main.cpp fdf4d62ea025e24b30a0f961fe16b87a1baf2bae Diff: https://git.reviewboard.kde.org/r/123463/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 123453: ki18n: Remove mention of language skipping
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123453/#review79331 --- Ship it! Ship It! - Chusslove Illich On Април 21, 2015, 9:25 по п., Lasse Liehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123453/ --- (Updated Април 21, 2015, 9:25 по п.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- This removes mention of a feature that no longer exists. Since commit 9f0e0e428dbf0626660a911fdb76d7544ea8beac in April 2014 Ki18n has not skipped a language if application domain is not translated for that language. Diffs - docs/programmers-guide.md 66629dc Diff: https://git.reviewboard.kde.org/r/123453/diff/ Testing --- None. Thanks, Lasse Liehu ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79325 --- src/widgets/accessmanager.cpp (line 255) https://git.reviewboard.kde.org/r/123443/#comment54156 IMO you should always verify if the data is readable, because not all lib users are trustworthy (assert isn't meant for input validation of untrustworthy callers - public api) - Emmanuel Pescosta On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 1:46 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 7:13 a.m.) Review request for KDE Frameworks. Changes --- Try to properly fix the isAvailable crash Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs (updated) - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/#review79327 --- Ship it! Ship It! - Martin Gräßlin On April 22, 2015, 9:13 a.m., Nerdopolis Turfwalker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 9:13 a.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 22, 2015, 7:27 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit d6133737bd42b7d9d4f470fba5aa794cc9257748 by Martin Gräßlin on behalf of Nerdopolis Turfwalker to branch master. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Build failed in Jenkins: kinfocenter_stable_qt5 #16
See http://build.kde.org/job/kinfocenter_stable_qt5/16/changes
Changes:
[scripty] SVN_SILENT made messages (.desktop file)
--
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/kinfocenter_stable_qt5/ws/
Running Prebuild steps
[kinfocenter_stable_qt5] $ /bin/sh -xe /tmp/hudson2676066235438221582.sh
+ /home/jenkins/scripts/setup-env.sh
Preparing to perform KDE Continuous Integration build
== Setting Up Sources
From git://anongit.kde.org/kinfocenter
78c0b5d..0987a11 Plasma/5.3 - origin/Plasma/5.3
Branch jenkins set up to track remote branch Plasma/5.3 from origin.
== Cleaning Source Tree
HEAD is now at 78c0b5d SVN_SILENT made messages (.desktop file)
Removing build/
Success build forhudson.tasks.Shell@51928209
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/kinfocenter # timeout=10
Fetching upstream changes from git://anongit.kde.org/kinfocenter
git --version # timeout=10
git -c core.askpass=true fetch --tags --progress
git://anongit.kde.org/kinfocenter +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 0987a11f9cd238a5a88361687a96093f3b1f4f60
(refs/heads/jenkins)
git config core.sparsecheckout # timeout=10
git checkout -f 0987a11f9cd238a5a88361687a96093f3b1f4f60
git rev-list 78c0b5de143cee1894595f5cdd64811e12cb48b6 # timeout=10
git tag -a -f -m Jenkins Build #16 jenkins-kinfocenter_stable_qt5-16 #
timeout=10
[kinfocenter_stable_qt5] $ /bin/sh -xe /tmp/hudson3213618127029402905.sh
+ /home/jenkins/scripts/execute-job.sh
KDE Continuous Integration Build
== Building Project: kinfocenter - Branch Plasma/5.3
== Build Dependencies:
ki18n - Branch master
kdewebkit - Branch master
kunitconversion - Branch master
attica - Branch master
libgit2 - Branch master
kdbusaddons - Branch master
qt5 - Branch 5.4.2
kitemmodels - Branch master
kdelibs4support - Branch master
kemoticons - Branch master
kcompletion - Branch master
kdesignerplugin - Branch master
knotifications - Branch master
kwindowsystem - Branch master
kdesu - Branch master
kpty - Branch master
kguiaddons - Branch master
kxmlrpcclient - Branch master
kparts - Branch master
cmake - Branch master
kdnssd - Branch master
extra-cmake-modules - Branch master
kded - Branch master
ktexteditor - Branch master
kcmutils - Branch master
kdeclarative - Branch master
kauth - Branch master
kcoreaddons - Branch master
kxmlgui - Branch master
kjsembed - Branch master
knewstuff - Branch master
threadweaver - Branch master
kwallet - Branch master
kiconthemes - Branch master
kconfigwidgets - Branch master
kio - Branch master
frameworkintegration - Branch master
kdoctools - Branch master
solid - Branch master
sonnet - Branch master
kconfig - Branch master
kpackage - Branch master
kcodecs - Branch master
kwidgetsaddons - Branch master
dogtail - Branch master
polkit-qt-1 - Branch master
kwayland - Branch Plasma/5.3
plasma-framework - Branch master
kbookmarks - Branch master
kidletime - Branch master
kactivities - Branch master
karchive - Branch master
kjs - Branch master
kcrash - Branch master
kitemviews - Branch master
kjobwidgets - Branch master
kinit - Branch master
kglobalaccel - Branch master
kservice - Branch master
knotifyconfig - Branch master
kplotting - Branch master
kdesupport-svn - Branch master
kross - Branch master
khtml - Branch master
ktextwidgets - Branch master
phonon - 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
Re: Review Request 123453: ki18n: Remove mention of language skipping
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123453/ --- (Updated April 22, 2015, 8:32 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Chusslove Illich. Changes --- Submitted with commit a9c4abd5db5c0714a541e7c391659077ec8ae2f5 by Lasse Liehu to branch master. Repository: ki18n Description --- This removes mention of a feature that no longer exists. Since commit 9f0e0e428dbf0626660a911fdb76d7544ea8beac in April 2014 Ki18n has not skipped a language if application domain is not translated for that language. Diffs - docs/programmers-guide.md 66629dc Diff: https://git.reviewboard.kde.org/r/123453/diff/ Testing --- None. Thanks, Lasse Liehu ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123463: Use categorized logging in runtime component
On April 22, 2015, 10:49 a.m., Martin Klapetek wrote: src/runtime/logging_p.h, lines 21-23 https://git.reviewboard.kde.org/r/123463/diff/1/?file=362353#file362353line21 It's useful if this also has #include QDebug so that you don't have to include two headers in the code (QDebug and logging_p.h) to do qdebug output that's a nice suggestion! Will add it. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123463/#review79332 --- On April 22, 2015, 9:54 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123463/ --- (Updated April 22, 2015, 9:54 a.m.) Review request for KDE Frameworks and Martin Klapetek. Bugs: 346429 https://bugs.kde.org/show_bug.cgi?id=346429 Repository: kglobalaccel Description --- BUG: 346429 Diffs - src/runtime/CMakeLists.txt e639fa5df96859f844476492235909756220ae5d src/runtime/component.cpp 3b5bdf99d0d26c032f2315bc4c1e64de5d321ee7 src/runtime/globalshortcut.cpp 3fe7bd961d3cdb23857654b3abff630d5ec0c3ec src/runtime/globalshortcutsregistry.cpp 3e4d720e13f6cda8e1b2827c70fce22423474d75 src/runtime/kglobalaccel_mac.cpp daaa24c3ec75cf6e78c923083b824ff331d6fe75 src/runtime/kglobalaccel_win.cpp 7457e348502b6f9d8cbb71be37652cf10ecf6587 src/runtime/kglobalaccel_x11.cpp abee5bcaa9795436833bc0b7fa5b259e7c3f86ac src/runtime/kglobalacceld.cpp 4e7cb9df1da37311d6f9e098f4d5b217cad54fd9 src/runtime/logging.cpp PRE-CREATION src/runtime/logging_p.h PRE-CREATION src/runtime/main.cpp fdf4d62ea025e24b30a0f961fe16b87a1baf2bae Diff: https://git.reviewboard.kde.org/r/123463/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 123463: Use categorized logging in runtime component
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123463/ --- (Updated April 22, 2015, 9:03 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Martin Klapetek. Changes --- Submitted with commit 8cc732ecc066b5250e501c85b7e2615d47c828ec by Martin Gräßlin to branch master. Bugs: 346429 https://bugs.kde.org/show_bug.cgi?id=346429 Repository: kglobalaccel Description --- BUG: 346429 Diffs - src/runtime/CMakeLists.txt e639fa5df96859f844476492235909756220ae5d src/runtime/component.cpp 3b5bdf99d0d26c032f2315bc4c1e64de5d321ee7 src/runtime/globalshortcut.cpp 3fe7bd961d3cdb23857654b3abff630d5ec0c3ec src/runtime/globalshortcutsregistry.cpp 3e4d720e13f6cda8e1b2827c70fce22423474d75 src/runtime/kglobalaccel_mac.cpp daaa24c3ec75cf6e78c923083b824ff331d6fe75 src/runtime/kglobalaccel_win.cpp 7457e348502b6f9d8cbb71be37652cf10ecf6587 src/runtime/kglobalaccel_x11.cpp abee5bcaa9795436833bc0b7fa5b259e7c3f86ac src/runtime/kglobalacceld.cpp 4e7cb9df1da37311d6f9e098f4d5b217cad54fd9 src/runtime/logging.cpp PRE-CREATION src/runtime/logging_p.h PRE-CREATION src/runtime/main.cpp fdf4d62ea025e24b30a0f961fe16b87a1baf2bae Diff: https://git.reviewboard.kde.org/r/123463/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 123463: Use categorized logging in runtime component
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123463/#review79332 --- Ship it! src/runtime/logging_p.h (lines 21 - 23) https://git.reviewboard.kde.org/r/123463/#comment54175 It's useful if this also has #include QDebug so that you don't have to include two headers in the code (QDebug and logging_p.h) to do qdebug output - Martin Klapetek On April 22, 2015, 9:54 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123463/ --- (Updated April 22, 2015, 9:54 a.m.) Review request for KDE Frameworks and Martin Klapetek. Bugs: 346429 https://bugs.kde.org/show_bug.cgi?id=346429 Repository: kglobalaccel Description --- BUG: 346429 Diffs - src/runtime/CMakeLists.txt e639fa5df96859f844476492235909756220ae5d src/runtime/component.cpp 3b5bdf99d0d26c032f2315bc4c1e64de5d321ee7 src/runtime/globalshortcut.cpp 3fe7bd961d3cdb23857654b3abff630d5ec0c3ec src/runtime/globalshortcutsregistry.cpp 3e4d720e13f6cda8e1b2827c70fce22423474d75 src/runtime/kglobalaccel_mac.cpp daaa24c3ec75cf6e78c923083b824ff331d6fe75 src/runtime/kglobalaccel_win.cpp 7457e348502b6f9d8cbb71be37652cf10ecf6587 src/runtime/kglobalaccel_x11.cpp abee5bcaa9795436833bc0b7fa5b259e7c3f86ac src/runtime/kglobalacceld.cpp 4e7cb9df1da37311d6f9e098f4d5b217cad54fd9 src/runtime/logging.cpp PRE-CREATION src/runtime/logging_p.h PRE-CREATION src/runtime/main.cpp fdf4d62ea025e24b30a0f961fe16b87a1baf2bae Diff: https://git.reviewboard.kde.org/r/123463/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 123464: Drop dead code path to get compositingActive without QGuiApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123464/ --- Review request for KDE Frameworks. Repository: kwindowsystem Description --- This was a dead code path as it's part of KWindowSystemPrivateX11 which only gets instantiated if it's on xcb platform which implies a QGuiApplication. Simplify creating atoms in kwindowsystem_x11.cpp We always use QX11Info::display. Diffs - src/kwindowsystem_x11.cpp d2c13b15617e9da2939b4e470c5c0237889921b7 Diff: https://git.reviewboard.kde.org/r/123464/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 123443: Make it possible to call a put job from an IODevice
On April 22, 2015, 9 a.m., Emmanuel Pescosta wrote: src/widgets/accessmanager.cpp, line 255 https://git.reviewboard.kde.org/r/123443/diff/1/?file=362233#file362233line255 IMO you should always verify if the data is readable, because not all lib users are trustworthy (assert isn't meant for input validation of untrustworthy callers - public api) I can do it, but QNetworkAccessManager doesn't do that, and I'd say it's better to be as similar to QNAM as possible. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79325 --- On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 1:46 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
